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 22, 2026, 9:32 PM ET / 01:32 UTC.

Summary
The branch changes gog youtube videos list to request nine non-owner videos.list parts by default, adds a --parts flag, and adds unit coverage for default, override, and JSON behavior.

Reproducibility: not applicable. this is a feature/default-change PR rather than a bug report. Source inspection confirms the omitted-flag behavior would change on the PR branch.

Review metrics: 2 noteworthy metrics.

  • Default videos.list parts: 3 -> 9 requested parts. This is the compatibility-sensitive behavior change maintainers need to approve before merge.
  • Changed files: 2 files affected. The code and tests changed, but generated command docs were not updated for the new public flag.

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.
  • Resolve the default-compatibility decision by preserving the old default or getting explicit maintainer approval for broad-by-default.
  • Run formatting and regenerate command docs after the behavior is settled.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists tests only; before merge, 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] Merging as-is changes omitted-flag behavior from three videos.list parts to nine parts, which can increase response size and quota use for existing scripts.
  • [P1] The new --parts CLI surface and broad default need maintainer product/compatibility approval before the default should change.
  • [P1] External real behavior proof is missing; unit tests do not show that the broader part set works against a real YouTube response.

Maintainer options:

  1. Preserve the old default (recommended)
    Keep snippet,contentDetails,statistics as the omitted-flag behavior and add an explicit broad selector for archive use.
  2. Accept broad-by-default intentionally
    Maintainers can choose the larger default response/quota cost, 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 default parts/quota compatibility before automation should repair or merge it.

Security
Cleared: The diff only touches 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 new YouTube tests — internal/cmd/youtube_test.go:788
  • [P3] Regenerate the command docs for --parts — internal/cmd/youtube.go:95
Review details

Best possible solution:

Keep the old three-part default unless maintainers explicitly choose broad-by-default; add documented opt-in broad selection, regenerated command docs, and redacted real command output proof before merge.

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 the omitted-flag behavior would change on the PR branch.

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 opt-in broad selection or an explicit maintainer decision plus docs and real 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 every existing gog youtube videos list call from the historical three parts to nine parts. That changes default JSON shape and quota use for existing scripts, so keep the old default or add an explicit all-parts mode unless maintainers accept this compatibility break.
    Confidence: 0.86
  • [P3] Format the new YouTube tests — internal/cmd/youtube_test.go:788
    CI's make fmt-check failed because internal/cmd/youtube_test.go needs formatting, so this branch cannot pass the repository gate until the added tests are gofmt/gofumpt-formatted.
    Confidence: 0.99
  • [P3] Regenerate the command docs for --parts — internal/cmd/youtube.go:95
    The branch adds the public --parts flag, but the generated docs/commands/gog-youtube-videos-list.md on the PR head still has no --parts entry even though it is generated from gog schema --json. Regenerate command docs after settling the flag/default behavior.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority feature with a limited command surface but meaningful default-behavior compatibility risk.
  • add merge-risk: 🚨 compatibility: Existing gog youtube videos list invocations would request more API parts and emit larger JSON by default after merge.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • add 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 lists tests only; before merge, 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.

Label justifications:

  • P2: This is a normal-priority feature with a limited command surface but meaningful default-behavior compatibility risk.
  • merge-risk: 🚨 compatibility: Existing gog youtube videos list invocations would request more API parts and emit larger JSON by default after merge.
  • 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 lists tests only; before merge, 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:

  • Current main default: Current main has no Parts field on YouTubeVideosListCmd and calls Videos.List with only snippet, contentDetails, and statistics, so the PR changes existing default behavior. (internal/cmd/youtube.go:130, fc13b4147e20)
  • PR default/flag change: The PR adds youtubeVideoAllParts, a public --parts flag, and resolveParts() returning the broad part set when the flag is omitted. (internal/cmd/youtube.go:95, d55f184dafa0)
  • Formatting gate failure: The failing CI jobs report make fmt-check failed with formatting needed: internal/cmd/youtube_test.go. (internal/cmd/youtube_test.go:788, d55f184dafa0)
  • Generated docs are stale: The PR-head generated command page still says it is generated from gog schema --json and lists --my-rating, but has no --parts flag entry. (docs/commands/gog-youtube-videos-list.md:3, d55f184dafa0)
  • Real behavior proof check: The PR body lists unit-test coverage, and the discussion has no terminal output, logs, screenshot, recording, or linked artifact showing the changed command against a real YouTube response. (d55f184dafa0)
  • Feature history: Git history shows the YouTube videos command was introduced and then recently extended by merged YouTube work, including feat(youtube): add YouTube Data API v3 support and feat(youtube): read liked videos and playlist contents. (internal/cmd/youtube.go:70, 21e2c126ba3c)

Likely related people:

  • coeur-de-loup: Previously contributed the merged YouTube videos list/rating and playlist-items work that this PR builds on. (role: recent area contributor; confidence: high; commits: 21e2c126ba3c; files: internal/cmd/youtube.go, internal/cmd/youtube_test.go, docs/commands/gog-youtube-videos-list.md)
  • steipete: Authored recent YouTube list auth behavior and merged/followed up on the prior YouTube videos list expansion. (role: recent area contributor and merger; confidence: high; commits: 5e4daadbc929, 4cac149d75a7; files: internal/cmd/youtube.go, internal/cmd/youtube_test.go, internal/googleapi/youtube.go)
  • Kuldip Satpute: Introduced the YouTube Data API v3 command surface that includes youtube videos list. (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