fix(section-manager): skip candidates without a usable image instead of erroring (HNT-2757)

[mmiermans]

Summary

Fixes HNT-2757. The Section Manager lambda was dropping ~2,800 section-item candidates/day with a client-side TypeGuardError: invalid type on $input.imageUrl, expect to be string, value: undefined (thrown by the typia assert in mapSqsSectionItemToCreateApprovedItemApiInput). Because it throws before calling admin-api it is invisible to the hnt-admin-api error alert, but it is the single largest section-ingestion failure (~79% of that lambda's ~3.5% failure rate).

Two root causes, both addressed:

1. ML sends candidates with no image (image_url: null/empty). The ML model treats image_url as optional; a strict downstream assert then turned every image-less candidate into a hard error. That is expected data, not an actionable failure.
2. A present image is wrongly rejected by validateImageUrl, which routes through the pocket-image-cache (Thumbor) proxy and drops the item if the proxy responds non-OK.

Reproduction (local fetch)

Host (from prod failures)	direct fetch	+ browser UA	pocket-image-cache proxy (validateImageUrl)
ilsole24ore.com	200	200	500
pap.pl	200	200	400
image.ie	403	403	404
rollingstone.de / ppe.pl	200	200	200

ilsole24ore.com images are directly fetchable (200) but the proxy returns 500, so validateImageUrl rejected valid items. A User-Agent does not help (the failure is proxy-side). Some hosts (image.ie) block all datacenter fetches regardless.

Change

• image_url missing/empty → skip the candidate as an expected no-op (info log; counted as skipped, not failed). Images remain required, so image-less items are still not created.
• Removed the validateImageUrl (pocket-image-cache) pre-check for a present image. It produced false negatives (above) and is redundant with curated-corpus-api, which performs the authoritative image fetch + S3 upload. Genuine unfetchable images (e.g. image.ie) now surface at that layer as the "Could not generate an S3 URL" error tracked by HNT-2758, instead of as a silent section-manager crash.
• Added a skipped count to the run summary log.

Trade-off / scope

This converts silent, noisy section-manager drops into either (a) recovered items (proxy false-negatives like ilsole24ore) or (b) failures surfaced at the authoritative curated-corpus-api/S3 layer (HNT-2758). Net: fewer total drops and no more TypeGuardError noise; a portion of genuinely-unfetchable images shifts to HNT-2758, where coarse handling of that error is being addressed separately. Scoped to section-manager; corpus-scheduler is unchanged (it shares validateImageUrl but has ~0 such failures).

Test plan

• npx tsc --noEmit — clean.
• utils.spec.ts — 23/23 pass, incl. new tests: mapper returns null (skip) on null/empty image; processSqsSectionData counts a no-image candidate as skipped, not failed.
• index.spec.ts — pass.
• Note: 2 failures in validators.spec.ts are pre-existing on main (reproduce with this branch stashed; that file imports ./validators, not utils.ts; likely a typia/Node-24 local-env quirk), not introduced here.

Refs: HNT-2757 (this), HNT-2758 (S3 image fetch handling).

[claude]

Claude finished @mmiermans's task in 1m 26s —— View job

---

Code review: PR #396 — skip image-less candidates instead of erroring

• Read the diff and surrounding code
• Analyze correctness, edge cases, tests
• Post review feedback

---

Overall this is a well-reasoned, well-tested fix. The root-cause analysis in the description is thorough, the change is scoped tightly to section-manager, and the tests cover the new behavior (null image, empty-string image, and the processSqsSectionData skip accounting). I have no blocking concerns — a few minor/optional observations below.

Correctness ✅

• sqsSectionItem.image_url?.trim() || undefined correctly collapses null, undefined, '', and whitespace-only into undefined. image_url is typed string | null (types.ts:27), so the optional chain is safe.
• The apiInput === null guard in processSqsSectionData (utils.ts:132) is placed in the correct branch — the only path that calls the mapper is the "create new ApprovedItem" branch. The existingItem and approvedCorpusItem paths don't touch the mapper, which is right (those items already have images).
• mapSqsSectionItemToCreateApprovedItemApiInput has exactly one caller (verified via grep), so widening the return to ... | null doesn't leave any unhandled call site.
• Removing validateImageUrl is behaviorally sound: it returned the original URL unchanged on success (validators.ts:28), so dropping it doesn't lose any URL transformation — curated-corpus-api does the authoritative fetch/upload. corpus-scheduler-lambda still imports validateImageUrl and is correctly left untouched.
• By the time we reach assert<CreateApprovedCorpusItemApiInput> (utils.ts:329), imageUrl is guaranteed a non-empty string, so the typia assert on that field can no longer throw. 👍

Minor / optional

1. Summary totals no longer sum to the candidate count. successfulCandidates + failedCandidates + skippedCandidates won't equal candidates.length, because the pre-existing createSectionItem no-op path (utils.ts:158-160) continues without incrementing any counter. This isn't introduced by this PR, but now that the summary log advertises a skipped count, the omission is more visible. Optional: fold the createSectionItem no-op into skippedCandidates too, so the three buckets are exhaustive.

2. Log volume. The per-candidate console.log in the mapper fires for every image-less candidate (~2,800/day per the description). That's intentional for visibility and is info-level, so it's acceptable — just flagging that it's a non-trivial bump in log volume. If it proves noisy, a single aggregated count in the run summary (already present) may be enough.

3. Test fragility (pre-existing pattern). The new processSqsSectionData test asserts on mockConsoleLog.mock.calls[1][0]. This works because the mapper is mocked (so the mapper's own skip-log doesn't fire) and there are no SectionItems to remove. It matches the existing style in the file, so no change needed — just noting the positional assertion is a bit brittle if surrounding logging changes.

Tests

The test updates look correct and cover the important cases: mapper returns null on null/empty image, and processSqsSectionData counts a no-image candidate as skipped (not failed, no Sentry, no console.error). Non-null assertions (result!) on the previously-non-null return paths are appropriate given the type widening.

Nice work — the trade-off (proxy false-negatives recovered vs. genuine unfetchable images shifting to HNT-2758) is clearly documented and reasonable.
• hnt-2757-section-manager-imageurl

[github-actions]

Plan Result (section-manager-lambda-cdk-production)

CI link

Plan: 0 to add, 1 to change, 0 to destroy.

• Update
  • aws_lambda_function.section-manager-sqs-lambda_D7365DAE

Change Result (Click me)

  # aws_lambda_function.section-manager-sqs-lambda_D7365DAE will be updated in-place
  ~ resource "aws_lambda_function" "section-manager-sqs-lambda_D7365DAE" {
        id                             = "SectionManagerLambda-Prod-SQS-Function"
      ~ qualified_arn                  = "arn:aws:lambda:us-east-1:996905175585:function:SectionManagerLambda-Prod-SQS-Function:94" -> (known after apply)
      ~ qualified_invoke_arn           = "arn:aws:apigateway:us-east-1:lambda:path/2015-03-31/functions/arn:aws:lambda:us-east-1:996905175585:function:SectionManagerLambda-Prod-SQS-Function:94/invocations" -> (known after apply)
        tags                           = {
            "app_code"       = "content"
            "component_code" = "content-sectionmanagerlambda"
            "env_code"       = "prod"
            "environment"    = "Prod"
            "service"        = "SectionManagerLambda"
        }
      ~ version                        = "94" -> (known after apply)
        # (20 unchanged attributes hidden)

      ~ environment {
          ~ variables = {
              - "GIT_SHA"     = "692d58d478b8683256dc914fbc0f654d45ef74a0" -> null
                # (5 unchanged elements hidden)
            }
        }

        # (4 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

⚠️ Errors

• failed to add a label section-manager-lambda-cdk-production/add-or-update: label name is too long (max: 50)