Repository navigation
fix: streamline local youtube access and frame selection - #214
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Independent review — 970d243Verdict: 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
Nonblocking P3 — partial metadata consistency
Independent checks in my own terminal
LimitsNo 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. |
Independent re-review — 81ee96fVerdict: 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
Independent checksUsing locked dependencies in my own isolated checkout:
Nonblocking P3 follow-ups
Verification limitsNo 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. |
Independent re-review — e291561Verdict: 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
Independent final-head verification
Remaining nonblocking documentation consistencyP3: LimitsNo 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. |
Independent re-review — f0bb89fVerdict: 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
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. |
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-storyboardswhen captions are insufficient.--transcript-onlyremains 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
No application deployment is required.