Skip to content

feat(slides): add native element operations#846

Merged
steipete merged 1 commit into
mainfrom
feat/slides-elements-826
Jun 19, 2026
Merged

feat(slides): add native element operations#846
steipete merged 1 commit into
mainfrom
feat/slides-elements-826

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

Summary

  • add slides element commands for shape/line creation, affine transforms, styling, z-order, grouping, alt text, and guarded deletion
  • validate geometry, object IDs, enum values, conflicting style/transform options, and destructive confirmation locally
  • add request-level regression coverage, generated command references, usage docs, changelog, and a live create/read/delete lifecycle

Closes #826.

Testing

  • make ci
  • focused go test ./internal/cmd -run 'TestSlidesElement' -count=1
  • CLI help and --dry-run --json request checks
  • autoreview: clean, no actionable findings
  • live Slides lifecycle attempted with [email protected]; blocked before API access because the encrypted file keyring requires GOG_KEYRING_PASSWORD in this non-interactive session

@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 19, 2026, 4:14 AM ET / 08:14 UTC.

Summary
The PR adds a gog slides element subtree for native shape/line creation, transforms, styling, z-order, grouping, alt text, and guarded deletion, plus docs, tests, and live-test script coverage.

Reproducibility: not applicable. this is a feature PR for a known missing command surface rather than a bug reproduction. Source inspection confirms current main lacks the slides element subtree while the PR adds it.

Review metrics: 2 noteworthy metrics.

  • Command surface: 9 subcommands added. The PR adds a broad user-facing Slides mutation subtree whose API semantics need maintainer review beyond compilation.
  • Changed footprint: 20 files, +1573/-1. The diff spans command code, tests, generated docs, hand-written docs, changelog, and live-test coverage.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #826
Summary: This PR is the candidate implementation for the canonical open issue tracking remaining native Slides element operations.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Run a redacted live Slides lifecycle after providing GOG_KEYRING_PASSWORD or another authenticated test setup.

Risk before merge

  • [P1] The PR adds new mutating Google Slides element operations, and the live lifecycle was blocked before API access, so request-construction tests and dry-run JSON do not prove provider behavior.
  • [P1] This is a new command surface in an already supported area; maintainer review should confirm the v1 command shape before merge.

Maintainer options:

  1. Run live Slides lifecycle before merge (recommended)
    Use an authenticated test account to prove create-shape, create-line, style, transform, z-order, group/ungroup, alt-text, and guarded deletion against a real deck before landing.
  2. Accept request-level confidence
    Maintainers can merge based on request-construction tests and dry-run output, but that accepts the risk that live Google Slides behavior differs from the fixtures.

Next step before merge

  • [P2] This collaborator-authored feature PR needs maintainer merge judgment and live provider validation, not an automated code repair.

Security
Cleared: The diff adds Go command/test code, generated docs, hand-written docs, and a live-test script without changing dependencies, workflows, credentials, install paths, or secret handling.

Review details

Best possible solution:

Keep this PR open for maintainer review, run the live Slides element lifecycle with the required keyring password or another authenticated account, then merge if the command surface and provider proof are accepted.

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

Not applicable: this is a feature PR for a known missing command surface rather than a bug reproduction. Source inspection confirms current main lacks the slides element subtree while the PR adds it.

Is this the best way to solve the issue?

Yes, with merge gating: a focused batchUpdate-backed element subtree matches the linked issue and existing Slides command patterns. The remaining requirement is maintainer acceptance plus live Google Slides proof after credentials are available.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority Slides capability improvement with clear linked issue context and limited blast radius.
  • add merge-risk: 🚨 other: Green CI does not prove these new mutating Google Slides element operations against the live provider, and the PR body says live API access was blocked.
  • 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: The external-contributor proof gate does not apply to this collaborator-authored PR, but the PR body says live Slides validation was blocked before API access, so maintainer live proof remains the merge-safety step.

Label justifications:

  • P2: This is a normal-priority Slides capability improvement with clear linked issue context and limited blast radius.
  • merge-risk: 🚨 other: Green CI does not prove these new mutating Google Slides element operations against the live provider, and the PR body says live API access was blocked.
  • 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: The external-contributor proof gate does not apply to this collaborator-authored PR, but the PR body says live Slides validation was blocked before API access, so maintainer live proof remains the merge-safety step.
Evidence reviewed

Acceptance criteria:

  • [P1] make ci.
  • [P1] go test ./internal/cmd -run 'TestSlidesElement' -count=1.
  • [P1] Run the Slides live-test lifecycle or equivalent redacted terminal proof with an authenticated Google account.

What I checked:

  • Repository policy read: AGENTS.md was read fully, and its PR review-mode guidance applies: inspect with read-only PR commands and do not switch branches or edit code. (AGENTS.md:1, b3c285613b74)
  • Current main lacks the new subtree: Current main registers the adjacent Slides structural, image, text, table, and style commands, but it has no Element SlidesElementCmd entry. (internal/cmd/slides.go:25, b3c285613b74)
  • Latest release also lacks the subtree: The v0.28.0 Slides command registry has no slides element command, so this PR is not obsolete against the latest release. (internal/cmd/slides.go:20, 6bc67d7dd770)
  • PR implementation surface: The PR head adds SlidesElementCmd with create-shape, create-line, transform, style, z-order, group, ungroup, alt-text, and delete subcommands. (internal/cmd/slides_element.go:16, d4a56a69dbe3)
  • Proof gap: The PR body reports make ci, focused tests, dry-run request checks, and an attempted live Slides lifecycle that was blocked before API access by missing GOG_KEYRING_PASSWORD. (d4a56a69dbe3)
  • Recent Slides ownership history: Git blame and recent logs tie the current Slides command registry and adjacent native Slides features to recent area work by steipete and bot-assisted landed commits. (internal/cmd/slides.go:25, a91ceadc8c31)

Likely related people:

  • steipete: Git blame, PR discussion, and adjacent merged history show recent work on the Slides command registry, native table creation, structured introspection, and image support in the affected area. (role: recent area contributor; confidence: high; commits: c78e609203f9, 5ff811dcc498, 32a6dddf8d86; files: internal/cmd/slides.go, internal/cmd/slides_table.go, internal/cmd/slides_introspection.go)
  • sebsnyk: The linked canonical issue supplies the requested native element operations and command-shape examples this PR implements. (role: reporter and command-shape source; confidence: medium; files: docs/slides-structure.md, internal/cmd/slides_element.go)
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: 🐚 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. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jun 19, 2026
@steipete
steipete merged commit eda03e0 into main Jun 19, 2026
9 checks passed
@steipete
steipete deleted the feat/slides-elements-826 branch June 19, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. 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.

slides: no structural commands — create/duplicate/reorder native slides, create/move/style elements

1 participant