Skip to content

refactor(core): share page media resolution and composition - #416

Merged
steipete merged 1 commit into
mainfrom
codex/shared-page-media
Sep 5, 2026
Merged

refactor(core): share page media resolution and composition#416
steipete merged 1 commit into
mainfrom
codex/shared-page-media

Conversation

@steipete

@steipete steipete commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Shared page-media ownership

HTML and Firecrawl extraction previously repeated media detection, embedded-video policy, transcript resolution, source-metric refresh, content composition, and diagnostic shaping. A shared page-media module now owns those operations and their typed context; the fetch coordinator builds common options once.

The page builders retain their distinct Readability/Reddit, metadata, Markdown, empty-result, and video-only policies. Metrics still use the deadline captured at builder entry. Empty Firecrawl HTML is passed through unchanged without a metric refresh; raw HTML keeps its existing refresh behavior. Markdown conversion remains after transcript resolution.

Production code is 64 lines smaller. Eight new contract tests add coverage rather than trading away behavior for line count.

Proof

  • All 126 link-preview tests pass, including the new HTML/Firecrawl policy matrix, empty-input distinctions, and transcript/metrics/Markdown ordering.
  • Build and full project gate pass: 3,087 tests (43 skipped), 94.29% line and 85.21% branch coverage.
  • Independent Codex review: no actionable P0–P2 findings.

No dependency or public API changes. The new functions and context are private implementation boundaries.

Hosted CI also passes on the exact PR head: Node 24 gate, Chromium E2E, and Firefox smoke, with security checks green.

@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 12:01 AM ET / 04:01 UTC.

ClawSweeper review

What this changes

Shares media detection, transcript resolution, and article/transcript composition between HTML and Firecrawl extraction while preserving their separate extraction policies.

Merge readiness

Ready for maintainer review

Keep open: this is a useful internal refactor, and the duplicated orchestration remains on pinned main and in the latest release. No actionable correctness or security findings were identified.

Priority: P3
Reviewed head: 5f4697ac4f9b33dc11ef30ba4643de1cea2aa383

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused consolidation with preserved behavior and useful contract tests; no blocking defect was found.
Proof confidence 🌊 off-meta tidepool Not applicable: OWNER-authored internal refactoring is exempt from the contributor runtime-proof gate, and no material authority change triggers an exception; the supplied builder tests are supplemental validation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: OWNER-authored internal refactoring is exempt from the contributor runtime-proof gate, and no material authority change triggers an exception; the supplied builder tests are supplemental validation.
Evidence reviewed 9 items Repository guidance and review boundary: Read the full root AGENTS.md and found no nested AGENTS.md or matching maintainer notes under the affected directories. Applied the core-library boundary and read-only workflow; repository policy and OWNER authorship also preclude automatic closure.
Complete introduced patch and merge identity: Inspected the complete local patch, including the test content truncated in the supplied context. Raw commit records confirm the original head's pinned main parent and the test merge's ordered main/head parents; the test merge has the same tree as the PR head.
Preserved media and composition policies: The shared helper forwards the existing transcript options and builder-start timestamp, then calls the existing composition helpers. Firecrawl explicitly preserves its empty-HTML refresh suppression; raw HTML retains refresh behavior, and Markdown conversion still precedes composition.
Findings None None.
Security None None.

How this fits together

Summarize’s core extraction library turns fetched HTML or Firecrawl results into content for summarization. The shared media layer resolves embedded transcripts and combines them with article text and diagnostics.

flowchart TD
  A[Page URL and extraction options] --> B[Fetch coordinator]
  B --> C[HTML extraction]
  B --> D[Firecrawl extraction]
  C --> E[Shared media resolution]
  D --> E
  E --> F[Transcript and source metrics]
  F --> G[Article and transcript composition]
  G --> H[Extracted content and diagnostics]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +166/−233 (net −67); tests +161/−0; docs +3 The extraction removes duplicated orchestration while adding focused regression coverage.
Added contract cases 8 cases across HTML and Firecrawl The tests cover shared behavior alongside source-specific empty-input and ordering contracts.

Technical review

Best possible solution:

Keep shared media orchestration private to core while retaining source-specific extraction policies in the two builders.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR consolidates existing behavior rather than reporting a bug; source comparison and regression-test inspection found no introduced failure.

Is this the best way to solve the issue?

Yes: the shared helper reuses existing policy and composition functions while leaving HTML, Firecrawl, and Markdown-specific decisions with their current owners.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8308778d4e30.

Labels

Label changes:

  • add P3: This is internal code cleanup with no intended user-facing behavior change or urgent defect.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: OWNER-authored internal refactoring is exempt from the contributor runtime-proof gate, and no material authority change triggers an exception; the supplied builder tests are supplemental validation.

Label justifications:

  • P3: This is internal code cleanup with no intended user-facing behavior change or urgent defect.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: OWNER-authored internal refactoring is exempt from the contributor runtime-proof gate, and no material authority change triggers an exception; the supplied builder tests are supplemental validation.

Evidence

What I checked:

  • Repository guidance and review boundary: Read the full root AGENTS.md and found no nested AGENTS.md or matching maintainer notes under the affected directories. Applied the core-library boundary and read-only workflow; repository policy and OWNER authorship also preclude automatic closure. (AGENTS.md:10, 5f4697ac4f9b)
  • Complete introduced patch and merge identity: Inspected the complete local patch, including the test content truncated in the supplied context. Raw commit records confirm the original head's pinned main parent and the test merge's ordered main/head parents; the test merge has the same tree as the PR head. (5f4697ac4f9b)
  • Preserved media and composition policies: The shared helper forwards the existing transcript options and builder-start timestamp, then calls the existing composition helpers. Firecrawl explicitly preserves its empty-HTML refresh suppression; raw HTML retains refresh behavior, and Markdown conversion still precedes composition. (packages/core/src/content/link-preview/content/page-media.ts:25, 5f4697ac4f9b)
  • Coordinator preserves redirected and original media inputs: The HTML call overrides the shared context with effectiveUrl and passes mediaHtml separately from normalized article HTML. Firecrawl diagnostics remain a shared object, preserving updates made during fallback. (packages/core/src/content/link-preview/content/index.ts:448, 5f4697ac4f9b)
  • Main and release still contain separate orchestration: Pinned main's HTML and Firecrawl builders independently resolve transcripts and refresh metrics. The latest release, v0.21.11, also contains inline transcript resolution and embedded composition; neither tree contains page-media.ts. No merged replacement was established. (packages/core/src/content/link-preview/content/html.ts:140, 8308778d4e30)
  • Latest-release source check: Release v0.21.11 retains direct calls to resolveTranscriptForLink and selectEmbeddedVideoContent in the HTML builder. (packages/core/src/content/link-preview/content/html.ts:150, 1dcbff752de7)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

ClawSweeper status: review started.

I am starting a fresh review of this pull request: refactor(core): share page media resolution and composition This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete
steipete merged commit a940f48 into main Sep 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant