[MohamedBassem]

No description provided.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3559c817c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid marking partial probe metadata as reusable

When a retryable first attempt extracts fresh probe metadata for a bookmark that already has title/image fields, the COALESCE writes above leave the existing row values in place, but this line still records probeMetadataAt. A later retry then skips re-extraction and loadStoredProbeMetadata reads the stale existing row as if it were the fresh probe, so a blocked render can prefer old metadata over the clean preflight result. Only mark the probe as reusable if the current probe values are actually persisted separately or re-extract on retries when the fill-only update could not store them.

Useful? React with 👍 / 👎.

[greptile-apps]

Greptile Summary

This PR adds a "probe metadata" extraction pass to the crawler. When the crawler makes its existing preflight HTTP request to determine the URL's content type, it now also fetches the response body and runs it through the parse subprocess in a new metadataOnly mode. The extracted preview metadata (title, description, banner image, favicon) is written to the bookmark row immediately — before the browser finishes rendering — so users see a title and thumbnail much earlier. On a retry, previously stored probe metadata is reloaded from the database instead of re-fetching.

• A new metadataOnly flag in parseHtmlSubprocessIpc.ts and parseHtmlSubprocess.ts short-circuits the expensive readability extraction, returning only the lightweight metadata object.
• getContentType() is replaced by getContentTypeAndMetadata(), which races the fetch against a short header-timeout and, for HTML responses, fires a background metadata-extraction IIFE that returns a promise alongside the content type.
• A new probeMetadataAt timestamp column in bookmarkLinks tracks when a probe write succeeded, allowing retries to reload stored values via loadStoredProbeMetadata() instead of re-fetching. The writeProbeMetadata() helper writes metadata fill-only (COALESCE/NULLIF) so a delayed write from a failed attempt cannot clobber values written by a later retry.

Confidence Score: 4/5

The change is a well-structured, additive feature with careful fill-only DB writes and thorough challenge-page detection. The main subtlety to watch is that loadStoredProbeMetadata reads fields that the probe write never stored, and that two parse subprocesses can now run concurrently per worker.

The design is thoughtful — background probe promise, COALESCE fill-only writes, retry reuse via probeMetadataAt, and the isLikelyChallengePage heuristic are all well-reasoned. The inconsistency between what writeProbeMetadata stores and what loadStoredProbeMetadata reads is a real behavioral gap on retries for four metadata fields, and the doubled subprocess concurrency is an undocumented operational change, but neither causes data loss or broken functionality.

The orchestration in crawlerWorker.ts — specifically writeProbeMetadata, loadStoredProbeMetadata, and their interaction with resolveMetadata on retry paths — deserves the closest look.

Important Files Changed

Filename	Overview
apps/workers/workers/crawlerWorker.ts	Core of the feature: getContentTypeAndMetadata replaces getContentType, adds background metadata extraction IIFE, new writeProbeMetadata (fill-only DB write) and loadStoredProbeMetadata, plus orchestration in runCrawler. The two-subprocess-per-worker concurrency is a new resource consideration.
apps/workers/workers/utils/metadataResolver.ts	New utility: isLikelyChallengePage (title/body-marker heuristic for bot challenge detection) and resolveMetadata (probe-vs-render field merge). Well-guarded with size limits and exact-match semantics.
apps/workers/workers/utils/metadataResolver.test.ts	Good test coverage for both resolveMetadata and isLikelyChallengePage, including edge cases for empty strings, large documents, and blocked renders.
apps/workers/workers/utils/parseHtmlSubprocessIpc.ts	Adds optional metadataOnly boolean to the subprocess IPC input schema; change is backward-compatible.
apps/workers/scripts/parseHtmlSubprocess.ts	Two conditional guards added with !metadataOnly to skip readability extraction when in metadata-only mode; logic is straightforward and correct.
packages/db/drizzle/0086_bookmark_links_probe_metadata_at.sql	Nullable probeMetadataAt integer column added to bookmarkLinks; safe additive migration.
packages/db/schema.ts	Schema addition of probeMetadataAt timestamp field matches the migration; no other schema changes.
apps/workers/workerUtils.ts	Exposes probeMetadataAt from getBookmarkDetails; trivial one-line addition.
packages/shared-server/src/eventLogTypes.ts	Adds crawler.probe.metadata and crawler.render_blocked fields to the crawlerWorker.run event log type.
packages/shared/config.ts	CRAWLER_PREFLIGHT_USER_AGENT was already present; no net-new config changes in this PR.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/workers/workers/crawlerWorker.ts:1963-1999
**`loadStoredProbeMetadata` reads fields that `writeProbeMetadata` never stored**

`writeProbeMetadata` only persists `title`, `description`, `imageUrl`, and `favicon` (with fill-only COALESCE). `loadStoredProbeMetadata` also reads `author`, `publisher`, `datePublished`, and `dateModified` (lines 1975–1979). On a retry where `reuseStoredProbeMetadata=true`, those four fields will come from whatever was last written to the row by a previous attempt's Phase 1 render write — not from the original probe — so the "reloaded probe metadata" passed to `resolveMetadata` can silently include stale render values for those fields.

### Issue 2 of 2
apps/workers/workers/crawlerWorker.ts:1718-1727
**Concurrent parse subprocesses double per-worker memory pressure**

The probe now spawns a `metadataOnly` subprocess in the background while the browser crawl is running, and then a full subprocess is spawned again after the browser render. With `CRAWLER_NUM_WORKERS` workers, up to `2 × CRAWLER_NUM_WORKERS` parse subprocesses can be in flight simultaneously — each bounded by `CRAWLER_PARSER_MEM_LIMIT_MB` (default 512 MB). Operators who tuned that limit for single-subprocess concurrency may see elevated memory usage without a clear explanation in the docs or the config.

_{Reviews (1): Last reviewed commit: "feat: add extra metadata parse in the pr..." | Re-trigger Greptile}

[greptile-apps]

loadStoredProbeMetadata reads fields that writeProbeMetadata never stored

writeProbeMetadata only persists title, description, imageUrl, and favicon (with fill-only COALESCE). loadStoredProbeMetadata also reads author, publisher, datePublished, and dateModified (lines 1975–1979). On a retry where reuseStoredProbeMetadata=true, those four fields will come from whatever was last written to the row by a previous attempt's Phase 1 render write — not from the original probe — so the "reloaded probe metadata" passed to resolveMetadata can silently include stale render values for those fields.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/workers/workers/crawlerWorker.ts
Line: 1963-1999

Comment:
**`loadStoredProbeMetadata` reads fields that `writeProbeMetadata` never stored**

`writeProbeMetadata` only persists `title`, `description`, `imageUrl`, and `favicon` (with fill-only COALESCE). `loadStoredProbeMetadata` also reads `author`, `publisher`, `datePublished`, and `dateModified` (lines 1975–1979). On a retry where `reuseStoredProbeMetadata=true`, those four fields will come from whatever was last written to the row by a previous attempt's Phase 1 render write — not from the original probe — so the "reloaded probe metadata" passed to `resolveMetadata` can silently include stale render values for those fields.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Concurrent parse subprocesses double per-worker memory pressure

The probe now spawns a metadataOnly subprocess in the background while the browser crawl is running, and then a full subprocess is spawned again after the browser render. With CRAWLER_NUM_WORKERS workers, up to 2 × CRAWLER_NUM_WORKERS parse subprocesses can be in flight simultaneously — each bounded by CRAWLER_PARSER_MEM_LIMIT_MB (default 512 MB). Operators who tuned that limit for single-subprocess concurrency may see elevated memory usage without a clear explanation in the docs or the config.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/workers/workers/crawlerWorker.ts
Line: 1718-1727

Comment:
**Concurrent parse subprocesses double per-worker memory pressure**

The probe now spawns a `metadataOnly` subprocess in the background while the browser crawl is running, and then a full subprocess is spawned again after the browser render. With `CRAWLER_NUM_WORKERS` workers, up to `2 × CRAWLER_NUM_WORKERS` parse subprocesses can be in flight simultaneously — each bounded by `CRAWLER_PARSER_MEM_LIMIT_MB` (default 512 MB). Operators who tuned that limit for single-subprocess concurrency may see elevated memory usage without a clear explanation in the docs or the config.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!