[FagnerMartinsBrack]

Why

In the week to 2026-11-07 the fleet errors widget held 280 unusable-image errors, all from the thumbnail cascade. 67 of them (24%) were tracking endpoints fetched as if they were article images:

Host	Path	Count
www.facebook.com	/tr	52
hspx.hotstar.com	/v1/events/track/cp_page_view	3
www.sec.gov	/akam/13/pixel_ + 8 hex	9
www.turkishairlines.com	/akam/13/pixel_ + 8 hex	3

Each one cost up to 5 s of the thumbnail cascade. It could also end up as the article's thumbnail, or stay in the saved HTML, where the reader's browser would request it.

What changed

• Shared check: isProvenTrackingImageUrl in @packages/crawl-article matches only those four exact hostnames and paths, as the URL parser reads them. Query, fragment and default port don't affect a match. A lookalike host, a different path or a non-default port is not matched. The paths are not decoded a second time.
• Thumbnails: tracking URLs are dropped as candidates are collected, and the order of the remaining candidates is unchanged. When a caller passes its own list, tracking URLs are skipped without a fetch or an error log.
• Body images: tracking URLs in img src, img srcset and picture source srcset are never downloaded, and an image whose only sources are tracking URLs doesn't use one of the 20 image slots. Images with only data: or relative sources still count, as before.
• Saved HTML: strip-tracking-image-sources runs even when nothing was downloaded. It removes only the matching src and srcset entries, keeps the descriptors of the entries that remain, never leaves an empty src, and returns the HTML unchanged when nothing matches. When a <picture>'s fallback <img> loses its only src and a sibling <source> still has an allowed rendition, the <img> takes that rendition's URL.

finalize-article.ts is unchanged from main (commit a7c40c1). An earlier commit on this PR also failed an image-only save whose only image was a beacon, and dropped a crawler-resolved thumbnail that was a beacon. No observed save motivated either: all 67 errors were thumbnail-cascade fetches. CLAUDE.md lets a reader's capture fail only on the deterministic readability check. Both guards were removed.

Not changed: the other 213 unusable-image errors (logos, QR codes, SVGs, CIA photos). Those can't be safely classified from the URL alone, so this PR doesn't touch them, and it does not claim to clear all 280. Stored articles are not rewritten either.

Validation

• The pre-commit pnpm check passed on each commit.
• The tests cover the plan's full boundary table, a fixture page for each case, and saved HTML with both an empty and a populated downloaded-media list.

After deploy

• Do one deterministic staging save with a fixture page and check that no tracking URL appears in the saved metadata or HTML.
• Then compare the errors widget against the 67-error baseline, after confirming the new build has handled traffic.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QeLuXYjTyQzYesHMBs3wxT

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: fix(save-link,@packages/crawl-article,@packages/finalize-article): never fetch or keep proven tracking pixels as article images #1337
• Commit: 3db87c2

[claude]

Claude finished @FagnerMartinsBrack's task in 38s —— View job

---

I'll analyze this and get back to you.

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. Every finalised article now gets one more full DOM parse

projects/save-link/src/runtime/domain/save-link/process-content-with-local-media.ts:18 calls stripTrackingImageSources on every article body, and that function runs parseHTML over the whole body. download-media.ts has already parsed the same HTML in extractImageGroups, and the parser had parsed it before that. Because the function returns the original string when nothing matches, the output is correct. The cost is one more linkedom parse per save, including saves with no tracking images. It is small for article-sized bodies. If you want to avoid it, a cheap substring check before parsing would skip the parse for nearly all bodies, for example checking whether the HTML contains any of the four hostnames.

2. A <picture> fallback can be left as <img> with no src

strip-tracking-image-sources.ts:45 keeps an <img> inside a <picture> whenever any source[srcset] remains, even after its src was removed. The test "keeps a picture's valid sibling sources…" expects exactly that output. It is fine when the remaining <source> has no media or type condition. When the only remaining source is type="image/webp" or has a media query, a browser that skips that source shows an image with no src. This is unlikely given the four evidenced hosts, so it only needs noting.

3. Image-only failure reason

finalize-article.ts:247 returns "Image resource is a known tracking endpoint" as the ok: false reason. That matches the existing reason strings in this file, so it is consistent. It is worth confirming that the code downstream that turns reason into the crawl's failed state, and into anything a reader sees, treats it like the other image failures. It should not be surfaced as raw text.

4. Checked and found correct

• isProvenTrackingImageUrl uses an exact hostname and path, and rejects any explicit port, so lookalike hosts are not matched.
• Relative and protocol-relative URLs never reach it unresolved: candidate collection resolves them, and the article parser resolves body URLs (resolve-relative-urls.ts).
• withoutTrackingImages drops only groups that had sources and lost all of them, so groups with only data: or relative sources keep their slot.
• withoutTrackingImage covers both resolvedThumbnail and the cascade, and firstUsableCandidate picks up the URL that was moved to provenUnusable.
• No new comments were added, there are no as casts, and assert is used for the selector invariants.

[FagnerMartinsBrack]

@claude address al low prio issues

[claude]

Claude finished @FagnerMartinsBrack's task in 3m 38s —— View job

---

Low-priority review items

I fixed item 2, made a smaller fix than suggested for item 1, and item 3 needed no change. All of it is in commit 7b308389b04, pushed to this branch. The pre-commit check passed on that commit: all tests passed and every coverage threshold was met.

• Read the changed code
• 1. Extra DOM parse on every save
• 2. <picture> fallback <img> left without src
• 3. How the image-only failure reason reaches readers
• Verify, commit, push

1. Extra parse — only partly addressed. I did not use the suggested hostname substring check. The PR's own tests treat https://www.faceboo%6B.com/tr as a tracking pixel, because the URL parser decodes it to www.facebook.com, and a substring check on the raw HTML would miss it. Character references (.), dot segments (/x/../tr) and backslashes would get past it the same way. So a check on hostnames could skip the parse for a body that does contain a tracking image.

Instead, stripTrackingImageSources now returns the body without parsing it when the HTML contains neither <img nor <source (case-insensitive). I confirmed that linkedom does not turn <image> into img, so this never skips a body that has a matching image. The catch is that it only saves the parse for bodies with no images, not for most saves. Removing the parse altogether would mean reusing the parse from download-media, which needs a bigger restructure. I left that out, since nobody has measured what the extra parse costs.

2. <picture> fallback — fixed. When the fallback <img> loses its src to a tracking pixel but a <source> still has an allowed image, the <img> now uses the first URL from that source's srcset as its src. The CDN rewrite then points it at the downloaded copy, like any other image. The two tests that expected <img> now expect the src, and a new test checks that a body with no img or source comes back unchanged.

3. Failure reason — no change needed. The reason is stored as { kind: "parse-error", detail } (in commit-deferred-crawl.ts), the same way as "Plain-text response is empty". failed-crawl-notice.ts picks what the reader sees from kind only, so readers get the usual failed notice and never see the detail text. It only goes to the parse-error log.

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: fix(save-link,@packages/crawl-article,@packages/finalize-article): never fetch or keep proven tracking pixels as article images #1337
• Commit: 7b30838

[claude]

Claude finished @FagnerMartinsBrack's task in 42s —— View job

---

I'll analyze this and get back to you.

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. The <picture> fallback src can come from a source the browser can't decode

projects/save-link/src/runtime/domain/save-link/strip-tracking-image-sources.ts:49 sets the fallback <img>'s src from the first source[srcset] in the picture. It does not look at that source's type or media. In the test at line 59, the fallback becomes figure.webp, which comes from a type="image/webp" source. A browser shows the <img> src only when it has rejected every <source>. So if the chosen source is type="image/avif", a browser without AVIF support rejects that source and then loads the same AVIF file through src, which leaves a broken image.

The four tracking hosts make this unlikely, so I'm only noting it. A cheap fix is to prefer the first remaining source that has no type attribute, and fall back to the first source when every one has a type.

2. A new test uses a negative assertion

download-media.test.ts:429 ends with expect(fakeFetch).not.toHaveBeenCalled(), and the test-driven-design skill asks us to avoid negative assertions. The file already uses this pattern in several places, and here the expected result really is "no fetch". The positive form states the same thing and would read like the sibling test above it: expect(fakeFetch.mock.calls.map(([url]) => url)).toEqual([]).

3. Already checked

• isProvenTrackingImageUrl matches only the exact hostname and path. It rejects any explicit port and any protocol other than http or https. It returns false when the URL doesn't parse, and it never decodes a path a second time.
• withoutTrackingImages drops only groups that had sources and lost all of them. Images with only data: or relative sources still use an image slot, and a test covers this (download-media.test.ts:423).
• stripTrackingImageSources runs before the CDN rewrite. So a fallback src taken from a <source> is rewritten to the downloaded copy whenever that rendition was downloaded, and otherwise stays at its origin, like any other image that wasn't downloaded.
• The early return html when no <img or <source appears, and the second one when nothing matched, keep unaffected bodies byte-for-byte.
• In finalizeImageArticle, the new ok: false branch fires only when no image survived and the fallback origin URL is a tracking URL. This is a fixed check on the URL, not the judge's verdict, so it doesn't conflict with the CLAUDE.md rule that a reader's own capture is never failed by the judge. As the earlier round confirmed, readers see the failure only through kind.
• Both commit messages follow Conventional Commits, with scopes that match the packages changed. No new comments were added, there are no as casts, and the invariants use assert.

[FagnerMartinsBrack]

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: fix(save-link,@packages/crawl-article,@packages/finalize-article): never fetch or keep proven tracking pixels as article images #1337
• Branch: plan-23-skip-tracking-image-endpoints
• Attempt: 1 of 5

[claude]

Claude finished @FagnerMartinsBrack's task in 18m 14s —— View job

---

Conflicts with main resolved

I rebased the branch onto origin/main (c1bc67e) and force-pushed it with lease. The head is now f4d146e5ede.

• Read the conflict-fixer instructions
• Read the PR's commit history
• Rebase onto origin/main and resolve conflicts
• Run pnpm check (one unrelated flaky e2e test failed, details below)
• Force-push with lease

What conflicted

Since this branch was cut, main has added c1bc67e fix: cap the thumbnail candidate walk at five paid fetches. It adds a fetched list to ThumbnailCascade and stops the cascade after five fetches.

• src/packages/crawl-article/src/extract-thumbnail.ts: both changes are kept. The tracking-URL skip runs before the five-fetch cap, so a tracking URL is still never fetched and never uses up one of the five fetches. It is still added to provenUnusable, as the PR's test expects.
• src/packages/finalize-article/src/finalize-article.ts:
  • In the first commit, I adapted the PR's withoutTrackingImage guard to main's fetched-based firstUsableCandidate.
  • The last commit (keep an image-only save and a resolved thumbnail as before) removes those guards. There I took main's file as it is, so the PR still leaves this file unchanged from main, as the PR body says.

The diff against main is the same 12 files and the same +445/−5 as before.

Verification

• pnpm check passed for @packages/crawl-article, @packages/finalize-article and save-link, along with every other project except hutch.
• hutch:test-with-coverage had one failing e2e test: readlist-preferences-visual.e2e-local.ts › "keeps both senders ticked in quick succession once the page is reopened". That test covers Gmail readlist preferences, which this PR doesn't touch.
  • I re-ran that test alone 3 times and it passed all 3, so it's a flake.
  • Because it failed, nx skipped hutch:check, so I haven't seen a full hutch check pass on the rebased branch. CI will run it.