Skip to content

feat(ai-research): keep v4 url history per team month - #117983

Merged
trunk-io[bot] merged 1 commit into
nico/ai-research-image-fetch-v4-storefrom
nico/ai-research-image-fetch-v4-history
Oct 23, 2026
Merged

trunk-io[bot] merged 1 commit into
nico/ai-research-image-fetch-v4-storefrom
nico/ai-research-image-fetch-v4-history

Conversation

@nicowaltz

@nicowaltz nicowaltz commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The v4 fetcher needs a crawl history to decide when to fetch a URL again and to honor robots.txt and tdmrep.
  • A v4 URL reference is scoped to a team month, and its stored image lives only as long as that month's key row. The v3 history does not fit that lifetime, and the v4 lane must not touch v3 tables.

Changes

  • The v4 lane gets its own crawl history, copied from the v3 fetcher and cut to v4 references.
  • It lives in one DynamoDB table of its own. The table holds the URL history of each imageurl:v4 reference and the robots.txt and tdmrep entries of each origin.
  • The lane reads and writes no v3 crawl history table.
  • A v4 reference is fetched again after 30 days or the response's freshness, whichever is later. It is revalidated with ETag or Last-Modified.
  • An entry expires with the key row of its team month, because the lane refreshes the stored image from it.
  • An entry records storedAtMs, when the lane last stored or refreshed the image, so the lane knows when a stored copy needs a refresh.

How did you test this code?

  • jest dynamodb-crawl-history url-history-expiry: batching, retries, malformed items, chunked configuration bodies, access checks, and the v4 refetch and expiry rule.
  • The DynamoDB integration test needs a local endpoint and was not run.

Test rationale: a v4 history entry that outlived its key would keep a URL marked fetched after its image expired.

Release status

  • No feature flag controls this change

Docs update

None. The format is in products/ai_training/docs/replay-data.md (#117905).

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5 (1M context)

  • Node stack for the v4 image fetcher. Spec: chore(ai-training): document the v4 replay dataset #117905.
  • The PR author set the design in a planning session: an unscrubbed v4 dataset recorded for a 10% md5 sample, sealed with the v3 envelope, anonymized downstream; the recorder in Rust as a byte-level linear scan; the fetcher as a duplicate of the Node lane.
  • Skills: /stacking-prs, /writing-tests, /writing-pr-descriptions.
  • No duplicate: no open PR implements v4.
  • Public artifact: every fixture is invented; nothing comes from customer data.

🤖 Generated with Claude Code

@nicowaltz nicowaltz self-assigned this Oct 15, 2026
@nicowaltz
nicowaltz added this pull request to stack #117988 October 15, 2026 16:43
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

🚨 Trunk lane — universal lane

This PR is assigned to the universal lane. It cannot merge in parallel with other PRs, so it can take longer to merge. Ask dev-ex if you think this is wrong.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Duplication (TypeScript) — 16 new duplicated blocks (worst 1812 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.test.ts:156 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.test.ts:129 210 1812
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:151 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:132 138 899
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.test.ts:6 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.test.ts:5 91 714
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:442 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:402 65 441
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.integration.test.ts:1 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.integration.test.ts:1 66 438
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:37 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:31 47 282
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.test.ts:106 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.test.ts:92 25 213
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:324 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:291 25 209
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:304 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:271 21 175
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:122 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:103 30 162
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:398 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:361 24 156
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/crawl-history.ts:25 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/crawl-history.ts:19 28 120
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:83 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:77 14 109
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:362 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:328 21 102
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:97 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:90 20 84
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts:348 nodejs/src/ingestion/pipelines/sessionreplay/ml-mirror-image-fetch/dynamodb-crawl-history.ts:315 13 81
✅ Comment density — 2% of added code lines are comments (20 of 975)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/dynamodb-crawl-history.ts 9 471
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/url-history-expiry.ts 7 24
nodejs/src/ingestion/pipelines/sessionreplay/ai-research/image-fetch-v4/crawl-history.ts 4 46

This check does not block merging. It updates on every push and clears when the share drops.

@greptile-apps

greptile-apps Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in Greptile

[Medium impact] Changes image fetch history expiry logic for a new URL format.

The missing-result convention must be fixed before merging; the cache test gap is non-blocking.

Reviews (1) · Last reviewed commit: "feat(ml-image-fetch): keep v4 url histor..." · Reviewed by Greptile

@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from b2adf74 to f3fb18e Compare October 16, 2026 09:37
@nicowaltz
nicowaltz marked this pull request as ready for review October 16, 2026 09:42
@parameterai

parameterai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Risk: No findings

This increment hardens the v4 crawl-history store: malformed DynamoDB rows now read as cache misses with a warning instead of failing the batch, legacy rows carrying a plaintext response URL are stripped on read so their validators read as unbound, the cache field becomes a keyed hash (responseUrlHmac), URL-history entries now expire 8 days after their team month instead of with the month key row, and fetched zstd bodies must be exactly one complete frame. All behavior changes fail toward refetching rather than skipping, and no exploitable path was found.

Sentinel reviewed 05760b0 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 16, 2026 09:42

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PostHog Review · Standard · reviewed f3fb18e: no findings.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

@trunk-io

trunk-io Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from f3fb18e to 30969e6 Compare October 16, 2026 12:45
@nicowaltz nicowaltz changed the title feat(ml-image-fetch): keep v4 url history per team day feat(ml-image-fetch): keep v4 url history per team month Oct 19, 2026
@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 30969e6 to 7b85b05 Compare October 19, 2026 13:54
@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 7b85b05 to 7135717 Compare October 19, 2026 15:39
@nicowaltz nicowaltz changed the title feat(ml-image-fetch): keep v4 url history per team month feat(ai-research): keep v4 url history per team month Oct 21, 2026
@nicowaltz
nicowaltz removed this pull request from stack #117988 October 21, 2026 14:45
@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 7135717 to 446e6d3 Compare October 21, 2026 15:18

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PostHog Review · Standard · reviewed 446e6d3: 1 Should fix.

@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 446e6d3 to dbfe1fe Compare October 22, 2026 13:51
@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from dbfe1fe to 173603c Compare October 22, 2026 15:49
@robbie-c

Copy link
Copy Markdown
Member

Written by Claude on Robbie's behalf, from a QA review of the v4 stack. These are minor findings: none of them blocks the merge.

  • One malformed history item crash-loops the lane and makes rollback unsafe. parseStoredItem in dynamodb-crawl-history.ts throws on any item that fails its checks. The throw fails read(), the batch and the pod, and the replay reads the same item again. A newer build that writes a new kind or status would also crash-loop an older build after a rollback. Suggestion: treat an item that does not parse as a cache miss, with a log line and a counter.

@robbie-c

Copy link
Copy Markdown
Member

Written by Claude on Robbie's behalf, from a QA review of the v4 stack.

The fetch history keeps the full image URL in plaintext for months.

  • cacheMetadata in image-fetcher.ts sets responseUrl to the requested URL. FetchRunner.terminal writes the whole cache object as JSON into ai-research-v4-image-fetch-history, under the key imageurl:v4:{team}:{month}:{hash}. So each row links a team to a raw image URL, query string included.
  • The rows have a TTL, but it is the team month key expiry (urlHistoryStorageExpiresAtMs returns teamMonthKeyExpiresAtMs): the start of the fifth month after the key month. That is 4 to 5 months, against 38 days for the images themselves.
  • A shred does not reach this table, so the URL stays readable after its key is gone. The infra README says the opposite: "a shredded month key makes them unlinkable to a URL".
  • v3 does neither. Its HttpCacheMetadata has no responseUrl, and its history for a month-scoped ref expires 8 days after the end of that month.

responseUrl is read in one place: the equality check that decides whether to send validators (previousCache?.responseUrl === url). Suggestions:

  1. Store an HMAC of the URL (the lane already holds the team month key) or a SHA-256 of it, and compare that. Or drop the field and send no validators after a redirect.
  2. Use v3's expiry for v4 refs: the end of the ref's month plus 8 days. A ref carries the month of its write, so jobs for it stop when that month ends, and nothing reads the item after that.

With both, the table holds no URL, and each item goes away a few weeks after its month. Fixing this before the first sync avoids a table scan to clean up rows already written.

@robbie-c robbie-c left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved with comments

@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 173603c to 05760b0 Compare October 23, 2026 14:15

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PostHog Review · Standard · reviewed 05760b0: 1 Should fix.

Comment on lines +26 to +27
const [year, month] = v4.keyMonth.split('-').map(Number)
return Date.UTC(year, month, 1 + MONTH_GRACE_DAYS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 · Retain URL history until the team-month key expires

For an October reference, this returns November 9, while teamMonthKeyExpiresAtMs('2026-10') returns March 1. The history therefore loses storedAtMs and validators while the key remains valid, even before a mid-October fetch reaches its 30-day refetch date. Late jobs fetch again instead of reusing that history; use teamMonthKeyExpiresAtMs(v4.keyMonth) for storage expiry.

Gives the v4 fetch lane its own crawl history, copied from the v3
fetcher and cut to v4 references. It lives in one DynamoDB table of
its own, which holds the URL history of each imageurl:v4 reference
and the robots.txt and tdmrep entries of each origin. The lane reads
and writes no v3 crawl history table.

- A v4 reference is fetched again after the crawl history TTL (30
  days) or the response's freshness, whichever is later, and is
  revalidated with ETag or Last-Modified.
- Its entry expires 8 days after the end of its month, as in v3. A
  reference carries the month it was written in, so no job for it
  comes after that.
- An item that does not parse reads as a cache miss, with a log line
  and a counter, so one bad row cannot fail every batch.
- An entry holds a keyed hash of the response URL, never the URL.
- An entry records storedAtMs, when the lane last stored or refreshed
  the image of the reference.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@nicowaltz
nicowaltz force-pushed the nico/ai-research-image-fetch-v4-history branch from 05760b0 to 8abad90 Compare October 23, 2026 16:09
@nicowaltz
nicowaltz added this pull request to stack #122764 October 23, 2026 16:13
@trunk-io
trunk-io Bot merged commit 1574876 into master Oct 23, 2026
210 checks passed
@trunk-io
trunk-io Bot deleted the nico/ai-research-image-fetch-v4-history branch October 23, 2026 17:11
@trunk-io

trunk-io Bot commented Oct 23, 2026

Copy link
Copy Markdown

This pull request was merged into master as part of stacked PR 119750.

@deployment-status-posthog

deployment-status-posthog Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-23 17:44 UTC Run
prod-us ✅ Deployed 2026-10-23 18:32 UTC Run
prod-eu ✅ Deployed 2026-10-23 18:30 UTC Run

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