Skip to content

youtube videos list: request all non-owner parts by default (+ --parts flag)#871

Open
coeur-de-loup wants to merge 1 commit into
openclaw:mainfrom
coeur-de-loup:feat/youtube-videos-list-parts
Open

youtube videos list: request all non-owner parts by default (+ --parts flag)#871
coeur-de-loup wants to merge 1 commit into
openclaw:mainfrom
coeur-de-loup:feat/youtube-videos-list-parts

Conversation

@coeur-de-loup

Copy link
Copy Markdown
Contributor

What

gog youtube videos list currently requests only three parts — snippet,contentDetails,statistics — so --json silently drops everything else the API returns for a non-owned video.

This makes the default request broad: every part readable for any non-owned video — snippet, contentDetails, statistics, status, topicDetails, recordingDetails, liveStreamingDetails, player, localizations — and adds a --parts flag to narrow it back down (e.g. --parts snippet,statistics).

Why

Tools that archive video metadata want lossless capture by default rather than having to enumerate parts. Default-broad + opt-in-narrow matches what read-only consumers generally expect.

Details

  • Owner-only parts (fileDetails, processingDetails, suggestions) are deliberately excluded from the default — the API 403s on them for videos the authenticated user doesn't own. They remain requestable via explicit --parts if you target your own uploads.
  • The API omits parts with no data for a given video (e.g. liveStreamingDetails on a non-live video), so the broad request tolerates per-video partial responses without erroring.
  • Behavior change: default output now includes more fields (and costs marginally more quota per call). Pass --parts snippet,contentDetails,statistics for the previous minimal behavior.

Tests

Adds coverage for: the default requests exactly the nine non-owner parts (never the three owner-only ones); --parts override narrows (incl. whitespace trimming); non-core fields (status.privacyStatus, topicDetails, all thumbnail sizes, liveStreamingDetails) survive into --json; and a video missing optional parts still serializes cleanly.

🤖 Generated with Claude Code

Extend 'yt videos list' so it fetches every videos.list part readable for
arbitrary (non-owned) videos — snippet, contentDetails, statistics, status,
topicDetails, recordingDetails, liveStreamingDetails, player, localizations —
instead of only the previous 3 (snippet,contentDetails,statistics). A new
--parts flag narrows the set when desired; the default is the full non-owner
list. Owner-only parts (fileDetails/processingDetails/suggestions) are
deliberately excluded — the API returns them only for the account's own uploads.

The Google SDK omits parts with no data for a given video (e.g.
liveStreamingDetails on a non-live video), so the broad request tolerates
per-video partial responses without erroring.

Tests assert the full part set is requested by default, --parts overrides it,
non-core part fields (status.privacyStatus, topicDetails, all thumbnail sizes,
liveStreamingDetails) survive in --json output, and a video missing optional
parts still serializes cleanly.
@clawsweeper

clawsweeper Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 1:07 PM ET / 17:07 UTC.

Summary
The PR changes gog youtube videos list to request nine non-owner videos.list parts by default, adds a --parts override, and adds unit tests for request parts and JSON serialization.

Reproducibility: not applicable. this is a feature/default-change PR rather than a bug report. Source inspection confirms current main uses the three-part request and the PR changes omitted-flag behavior.

Review metrics: 2 noteworthy metrics.

  • Default videos.list parts: 3 -> 9 requested parts. This is the compatibility-sensitive behavior maintainers need to approve before merge.
  • Public flag docs coverage: 1 public flag added, 0 docs files changed. The new CLI surface will be missing from generated command docs unless docs are regenerated after the behavior is settled.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal output or logs from a real gog youtube videos list --id ... --json run showing the broader fields; redact private IDs, tokens, endpoints, and account details.
  • Resolve the default compatibility decision by preserving the old default or getting explicit maintainer approval for broad-by-default.
  • Format the tests and regenerate command docs after the behavior direction is settled.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and comments list tests only; before merge, the contributor should add redacted terminal output or logs from a real gog youtube videos list run, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Existing invocations without --parts would move from three requested videos.list parts to nine, increasing response shape and quota/request cost for scripts that rely on the current default.
  • [P1] The PR has no redacted real YouTube command output or logs, so unit tests do not prove the broader part set works against live API responses.
  • [P1] The branch currently fails the formatting gate and omits generated command docs for the new public flag.

Maintainer options:

  1. Keep Broad Parts Opt-In (recommended)
    Preserve the current three-part omitted-flag behavior and add an explicit broad selector after the flag/default naming is approved.
  2. Accept Broad Default Deliberately
    Maintainers can choose the larger default response and quota behavior, but should require live proof and regenerated docs before landing.

Next step before merge

  • [P1] Needs contributor real-behavior proof and a maintainer decision on the broad-by-default compatibility change before automation should repair or merge it.

Security
Cleared: The diff only changes Go command/test code for a read-only YouTube request and adds no dependencies, workflow changes, secrets handling, or supply-chain surface.

Review findings

  • [P1] Keep broad YouTube parts opt-in until the default is approved — internal/cmd/youtube.go:105-107
  • [P3] Format the added YouTube tests — internal/cmd/youtube_test.go:788
  • [P3] Regenerate the command docs for --partsinternal/cmd/youtube.go:95
Review details

Best possible solution:

Preserve the existing three-part default unless maintainers intentionally approve broad-by-default; land a documented opt-in selector with redacted live YouTube proof.

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

Not applicable: this is a feature/default-change PR rather than a bug report. Source inspection confirms current main uses the three-part request and the PR changes omitted-flag behavior.

Is this the best way to solve the issue?

No: the implementation is focused, but broad-by-default changes existing output and quota behavior without maintainer approval. The safer path is preserving the old default or getting an explicit maintainer decision plus docs and live proof.

Full review comments:

  • [P1] Keep broad YouTube parts opt-in until the default is approved — internal/cmd/youtube.go:105-107
    When --parts is omitted, resolveParts switches existing gog youtube videos list calls from the historical three-part request to nine parts. Preserve the current default or require explicit maintainer approval for the larger response and quota behavior.
    Confidence: 0.88
  • [P3] Format the added YouTube tests — internal/cmd/youtube_test.go:788
    CI's make fmt-check reports formatting needed: internal/cmd/youtube_test.go, so the branch cannot pass the repository gate until the new tests are formatted.
    Confidence: 0.99
  • [P3] Regenerate the command docs for --partsinternal/cmd/youtube.go:95
    The PR adds a public CLI flag, but only code and tests changed; regenerate docs/commands/gog-youtube-videos-list.md after the default behavior is settled so users can discover --parts.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 213ddb60d7d1.

Label changes

Label justifications:

  • P2: This is a normal-priority YouTube CLI improvement with limited command scope but real compatibility risk for existing scripts.
  • merge-risk: 🚨 compatibility: Merging as-is changes omitted-flag behavior for existing gog youtube videos list users by requesting more API parts and emitting broader JSON.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and comments list tests only; before merge, the contributor should add redacted terminal output or logs from a real gog youtube videos list run, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its PR-link workflow guidance applies because this review used gh pr view and gh pr diff without changing branches or files. (AGENTS.md:36, 213ddb60d7d1)
  • Current main default: Current main has no Parts field on YouTubeVideosListCmd and calls Videos.List with only snippet, contentDetails, and statistics. (internal/cmd/youtube.go:130, 213ddb60d7d1)
  • PR default and flag change: The PR adds youtubeVideoAllParts, a public --parts flag, and resolveParts() returning the broad nine-part set when the flag is omitted. (internal/cmd/youtube.go:95, d55f184dafa0)
  • Product compatibility context: VISION.md says behavior changes that could break existing scripts should be discussed first, which applies to changing omitted-flag YouTube output and request size. (VISION.md:22, 213ddb60d7d1)
  • Generated docs stale: The generated command page documents current flags and says to run make docs-commands, but the PR changes only code and tests, so the new public --parts flag is not reflected in command docs. (docs/commands/gog-youtube-videos-list.md:3, 213ddb60d7d1)
  • Formatting check: GitHub CI's make fmt-check step reports formatting needed: internal/cmd/youtube_test.go. (internal/cmd/youtube_test.go, d55f184dafa0)

Likely related people:

  • steipete: Recent GitHub history shows repeated YouTube command refactors and fixes, including shared list infrastructure and account-auth behavior around this command surface. (role: recent area contributor; confidence: high; commits: 29cc4533ccb0, 7ae83c71793a, 5e4daadbc929; files: internal/cmd/youtube.go, internal/cmd/youtube_test.go, internal/googleapi/youtube.go)
  • coeur-de-loup: Previously contributed the merged YouTube liked-videos and playlist-items work that this PR builds on, including the current --my-rating path. (role: recent feature contributor; confidence: high; commits: 21e2c126ba3c; files: internal/cmd/youtube.go, internal/cmd/youtube_test.go, docs/commands/gog-youtube-videos-list.md)
  • satputekuldip: Introduced the original YouTube Data API v3 command group, including the videos listing surface now being changed. (role: introduced behavior; confidence: medium; commits: 05dd66607583; files: internal/cmd/youtube.go, internal/cmd/youtube_test.go, docs/commands/gog-youtube-videos-list.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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 added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant