Skip to content

fix: streamline local youtube access and frame selection - #214

Merged
devhims merged 6 commits into
mainfrom
fix/youtube-ctx-host-network
Oct 19, 2026
Merged

devhims merged 6 commits into
mainfrom
fix/youtube-ctx-host-network

Conversation

@devhims

@devhims devhims commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Problem / Motivation

Sandbox network failures could send accessible YouTube videos straight to the hosted fallback. Frame requests also downloaded storyboards even when timed captions located the relevant section. Empty captions could report partial results internally while the index appeared complete.

What changed

The skill retries sandbox network failures once before falling back to video2ctx. It prefers the narrowest available network permission, subject to environment approvals and access denials. Network errors are distinguished from confirmed video unavailability.

Indexing now starts with timed captions by default. The agent selects timestamps, extracts frames, and checks the images. Storyboards require --with-storyboards when captions are insufficient. --transcript-only remains a compatibility alias; combining both flags is rejected. Callers needing the previous combined index must opt in.

Index metadata propagates nested partial flags and warnings, including empty captions without warnings. Partial results retain their workspace for recovery.

The README, API overview, and skill guide describe caption-first indexing and optional storyboard downloads. The skill description also covers checking transcript misspellings and inaccuracies against video context. Direct-access guidance inspects error messages alongside codes and retryability. Missing-FFmpeg recovery tries storyboards when coarse imagery suffices. Exact frames still require FFmpeg. Delivered images are preserved before workspace cleanup.

Tests

  • 95 tests passed; 4 optional integration tests skipped.
  • Type, bundle consistency, skill validation, whitespace, and documentation build checks passed. Docs validation used Node 24 LTS.
  • Regression coverage verifies nested metadata and workspace preservation.
  • Live verification confirmed default indexing downloads no storyboards; host access recovered earlier sandbox failures.

No application deployment is required.

@vercel

vercel Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
video2ctx-web Ready Ready Preview Oct 19, 2026 7:08am UTC

@devhims devhims changed the title fix: retry youtube reads with host network access fix: streamline local youtube access and frame selection Oct 19, 2026

devhims commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Independent review — 970d243

Verdict: APPROVE. No blocking findings in the latest head. Reviewed both commits against cade5df, including the transcript-first change that arrived during review, the full diff, surrounding CLI/workflow/error handling and existing discussion.

Behavior reviewed

  • The network-error guidance matches the client's exhausted-client error aggregation. A single host retry uses the environment's permission mechanism; subsequent commands remain subject to permissions. A denial of the underlying action must still be respected, regardless of provider fallback.
  • --transcript-only skips storyboard retrieval while retaining the workspace marker/index contract used by frame extraction. Caption failures remain classified and incomplete workspaces are cleaned.
  • Existing frame timestamp/count validation is unchanged. The guide treats captions as candidate timing evidence and requires inspecting actual frames, with storyboard fallback when captions are insufficient.
  • Delivered images are preserved before temporary workspace cleanup; no application runtime/deployment change is introduced.

Nonblocking P3 — partial metadata consistency

packages/youtube-skills/src/watch/workflow.ts:107–129 does not propagate transcript.meta.partial to the index-level metadata. An offline empty-caption fixture returns segments: [] and nested partial: true, but index-level partial: false with no warnings. Ignoring this nested metadata predates this PR; the new mode exposes it without a storyboard warning. A consumer looking only at the top-level flag can mistake an empty caption read for complete evidence. Consider propagating transcript partial status and adding a regression test. This is nonblocking because the guide explicitly inspects segments and falls back when captions are insufficient; it is not a claim of a newly reproduced live Agent failure.

Independent checks in my own terminal

  • YouTube skills: 84 passed, 4 skipped.
  • Extraction library: 105 passed, 2 skipped.
  • Combined: 189 passed, 6 skipped.
  • Library build, skills TypeScript, generated-bundle consistency, bundled CLI help and whitespace checks passed.
  • An initial bundle check with shared dependency symlinks failed due to emitted dependency paths. Reinstalled locked dependencies in the isolated review checkout and reran successfully; no source changes were needed.
  • Verify CI 39208868220 succeeded for this exact head; Vercel status is successful.

Limits

No independent live YouTube/provider/model test, host-permission interaction replay or paid request was made. The PR author's live examples are reported evidence, not my own reproduction. The six optional tests remained skipped locally. Unit tests do not prove that every consuming agent will choose the correct escalation/fallback. No merge or deployment performed.

devhims commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Independent re-review — 81ee96f

Verdict: APPROVE. This supersedes the earlier 970d243 approval for current-head readiness. Reviewed the new default-mode change, complete PR diff and surrounding callers. Kiro independently reviewed this exact head and, after resolving two initially proposed documentation blockers, also approves with no blocking disagreement.

Latest change verified

  • Indexing defaults to captions only. --transcript-only remains an alias; --with-storyboards explicitly restores the combined scan.
  • Conflicting switches and valued forms are rejected before network/workspace creation.
  • Caption failure cleanup and extraction from a caption-only marked workspace are covered. Frame bounds/count validation is unchanged.
  • The private getWatchIndex callers are updated; this is not a change to the public all-things-youtube API. CLI users needing the previous combined default must opt in, as documented in the PR.

Independent checks

Using locked dependencies in my own isolated checkout:

  • Skills: 89 passed, 4 skipped.
  • Extraction library: 105 passed, 2 skipped.
  • Total 194 passed, 6 skipped.
  • Library build, skill TypeScript, generated bundle consistency and whitespace checks passed; tracked checkout clean.
  • Verify CI 39209259085 succeeded; Vercel status succeeded for this head.

Nonblocking P3 follow-ups

  1. packages/youtube-skills/src/watch/workflow.ts:107–129: nested transcript partial metadata is not propagated. Empty captions can have nested partial:true but index-level partial:false. This pre-existing inconsistency is more visible with the new default; consumers inspecting only top-level metadata may overestimate completeness. Preserve the workspace but propagate partial status in a follow-up.
  2. docs/skills/youtube-ctx.mdx:43 still describes always loading storyboard sheets for visual work. Align public prose with the new opt-in behavior.
  3. .agents/skills/youtube-ctx/references/visual.md:55: after missing FFmpeg, explicitly trying --with-storyboards when coarse imagery could answer the remaining question would improve recovery. Current wording permits only evidence-supported qualified answers and still requires exact-frame verification where necessary, so this is not a demonstrated incorrect-answer blocker. Storyboards cannot satisfy an explicit exact-frame request.
  4. Host-access guidance could explicitly prefer narrower network permission when the environment supports it. The existing environment-permission mechanism and require_escalated example are compatible with environments exposing that mechanism; no universal network-only option was established. All calls remain subject to the environment's approvals and underlying access denials.

Verification limits

No fresh independent live YouTube/model/paid request or host-approval interaction replay. Six optional tests were skipped locally. Kiro ran shipped CLI flag and sandbox-failure/cleanup probes, but registry access blocked its dependency install; the 194-test total is my independent result, not Kiro's. Agent instruction-following and live visual accuracy are not established by unit tests. No implementation, push, merge or deployment performed by this review.

devhims commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Independent re-review — e291561

Verdict: APPROVE. Re-reviewed the 81ee96f → e291561 changes against the earlier full review. Kiro independently reviewed this exact head and agrees: no blockers or regressions.

Prior notes resolved

  • packages/youtube-skills/src/watch/workflow.ts:106–131 now propagates video, transcript and storyboard partial flags independently of whether warning strings exist, collects warnings and deduplicates them. Empty-caption results retain their marked workspace for recovery; new tests pin this existing retention behavior.
  • The operative visual guide explicitly tries storyboards when FFmpeg is missing and coarse imagery could suffice, while preserving the incomplete status of exact-frame requests.
  • Permission guidance now prefers the narrowest supported network permission and explicitly respects approvals and underlying access denials.
  • The primary public workflow description matches transcript-first/opt-in storyboards.

Independent final-head verification

  • Skills: 95 passed, 4 skipped.
  • Extraction library: 105 passed, 2 skipped.
  • Total: 200 passed, 6 skipped.
  • Library build, skill TypeScript, generated-bundle consistency and whitespace checks passed using locked dependencies in my own checkout; tracked checkout clean.
  • Verify CI 39211141306 succeeded for this SHA; Vercel status succeeded.

Remaining nonblocking documentation consistency

P3: README.md:67, docs/api/agents.mdx:6 and docs/skills/youtube-ctx.mdx:23 still suggest a storyboard-first visual flow. Align those summaries with the actual caption-first default to avoid confusing readers about downloads and frame selection. references/direct.md:32 could also mention inspecting the error message alongside code/retryability, matching the parent skill's new network-retry guidance. These do not override the updated operative instructions and are not merge blockers.

Limits

No fresh live YouTube/model/paid request or host-approval interaction replay by this review. Six optional tests were skipped locally. Kiro's source/CLI probes and CI inspection are separate from the independent 200-test total; its dependency installation remained blocked by registry access. Local documentation build was not rerun; CI documentation checks passed. Unit/source results do not establish general live visual accuracy. No code changes, push, merge or deployment performed.

devhims commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Independent re-review — f0bb89f

Verdict: APPROVE. Reviewed the five-file documentation-only delta from e291561 and reconciled it with the previously tested implementation. Kiro reviewed this exact head and agrees; no remaining material findings or disagreement.

The README/API/skill summaries now consistently describe caption-first indexing and opt-in storyboards. Direct-access guidance now considers error messages and the parent skill's permission-aware retry. The new skill-description trigger for suspected transcript misspellings/misheard names asks for evidence-based checking, without promising audio retranscription or guaranteed corrections. Existing untrusted-content, visual-verification and permission safeguards remain intact.

Verification

  • Fresh local whitespace, skill-header, referenced-file existence and documented flag/error-guidance consistency checks passed.
  • Confirmed no runtime, tests, dependency or generated-bundle changes since e291561. The earlier independent 200 passed / 6 skipped result applies to that unchanged code; those suites were not rerun for this prose-only delta.
  • Verify CI 39214044742 and Vercel succeeded for the current SHA, including CI documentation validation.

No fresh live YouTube/model/paid request, local documentation build or host-approval replay. This review does not establish live correction accuracy. No code changes, push, merge or deployment performed.

@devhims
devhims merged commit 44933d2 into main Oct 19, 2026
10 checks passed
@devhims
devhims deleted the fix/youtube-ctx-host-network branch October 19, 2026 16:13

This branch was successfully deployed

1 active deployment
Preview — f0bb89f8 Deployed Oct 19, 2026 by vercel[bot]
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.

1 participant