fix(slides): add agent-friendly aliases for screenshot flags - #2156
Merged
Conversation
github-actions
Bot
added
the
size/M
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Slides screenshot shortcut adds hidden slide aliases. Shared parsing handles slide numbers and IDs across validation, dry-run, and execution. Tests cover precedence, invalid values, request generation, and live screenshot output. ChangesSlides screenshot aliases
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SlidesScreenshot
participant SelectorHelpers
participant ScreenshotAPI
CLI->>SlidesScreenshot: pass presentation and slide selectors
SlidesScreenshot->>SelectorHelpers: parse and normalize selectors
SelectorHelpers-->>SlidesScreenshot: return effective selectors
SlidesScreenshot->>ScreenshotAPI: build screenshot request
ScreenshotAPI-->>CLI: emit dry-run request or execute request
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/slides/slides_screenshot_test.go`:
- Around line 157-166: Update the error assertions in the slide validation test
to capture the metadata returned by errs.ProblemOf(err) and assert category is
errs.CategoryValidation, subtype is errs.SubtypeInvalidArgument, and param is
"--slide". Keep the existing *errs.ValidationError type assertion if needed, but
ensure all required typed metadata is verified through errs.ProblemOf.
In `@tests/cli_e2e/slides/slides_screenshot_dryrun_test.go`:
- Around line 51-78: Add a live E2E workflow alongside
TestSlidesScreenshotAliasesDryRunE2E that creates the required presentation and
slides, invokes slides +screenshot using the aliases with bot credentials,
verifies the saved screenshot output, and cleans up all created resources via
test cleanup. Keep the existing dry-run request-generation assertions unchanged
and make the live test self-contained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d78c342-a02e-48db-bfeb-d43f185994c8
📒 Files selected for processing (3)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.gotests/cli_e2e/slides/slides_screenshot_dryrun_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@806616b12ab08ac8671d133bcf216b22990d3ba8🧩 Skill updatenpx skills add larksuite/cli#fix/slides-screenshot-parameter-robustness -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2156 +/- ##
==========================================
+ Coverage 75.69% 75.70% +0.01%
==========================================
Files 942 944 +2
Lines 100079 100284 +205
==========================================
+ Hits 75750 75922 +172
- Misses 18537 18565 +28
- Partials 5792 5797 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BD-ZERO
force-pushed
the
fix/slides-screenshot-parameter-robustness
branch
from
August 3, 2026 12:14
82a83d0 to
9dece89
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/cli_e2e/slides/slides_screenshot_workflow_test.go`:
- Around line 48-53: Move the parentT.Cleanup registration in the slide
screenshot workflow to immediately after extracting and validating
presentationID, before extracting or asserting slideID. Ensure cleanup still
targets the created presentation so failures in slideID validation cannot leave
tenant data behind.
- Around line 95-97: Update the image inspection in the screenshot workflow test
to open imagePath through the repository’s internal/vfs API instead of os.Open,
while preserving the existing error assertion and deferred close. Remove the
now-unused os import; the fixture setup exception does not apply here.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5425750-f3b2-40e6-9b90-1f04f3b67dea
📒 Files selected for processing (4)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.gotests/cli_e2e/slides/slides_screenshot_dryrun_test.gotests/cli_e2e/slides/slides_screenshot_workflow_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/cli_e2e/slides/slides_screenshot_dryrun_test.go
- shortcuts/slides/slides_screenshot_test.go
- shortcuts/slides/slides_screenshot.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/slides/slides_screenshot.go`:
- Around line 288-291: The mixed-selector validation error must include its
parameter metadata and the E2E test must verify it. In
shortcuts/slides/slides_screenshot.go lines 288-291, update the error built in
the slideIDs/slideNumbers validation branch with a consistent WithParam value;
in tests/cli_e2e/slides/slides_screenshot_dryrun_test.go lines 97-100, assert
that result.Stderr contains the expected error.param value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d4ea7e5-2977-410b-9fce-bbaf80ec9fde
📒 Files selected for processing (6)
shortcuts/slides/shortcuts_alias_test.goshortcuts/slides/slides_add_slide.goshortcuts/slides/slides_delete_slide.goshortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.gotests/cli_e2e/slides/slides_screenshot_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/slides/slides_screenshot_test.go
6 tasks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/cli_e2e/slides/slides_screenshot_dryrun_test.go`:
- Around line 119-123: Update the dry-run validation assertions in the slides
screenshot test to verify the complete typed error envelope from result.Stderr:
assert the expected error.type, error.subtype, error.param, and error.message,
while preserving the exit-code 2 assertion. Also assert that result.Stdout is
empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1201b473-e85f-42f6-b7d3-764257f6b278
📒 Files selected for processing (3)
shortcuts/slides/slides_screenshot.goshortcuts/slides/slides_screenshot_test.gotests/cli_e2e/slides/slides_screenshot_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/slides/slides_screenshot.go
- shortcuts/slides/slides_screenshot_test.go
5 tasks
Accept common presentation and slide selector aliases while keeping canonical flags authoritative. Validate the ambiguous --slide alias using the SXSD short slide ID format and cover the behavior with unit and dry-run E2E tests.
Assert the full typed validation contract for ambiguous --slide values and add a self-contained bot workflow that creates a presentation, exercises screenshot aliases, verifies the saved image, and cleans up the resource.
BD-ZERO
force-pushed
the
fix/slides-screenshot-parameter-robustness
branch
from
August 4, 2026 02:52
a0f1a84 to
a340d43
Compare
fangshuyu-768
previously approved these changes
Aug 4, 2026
fangshuyu-768
approved these changes
Aug 4, 2026
6 tasks
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improve
slides +screenshotcompatibility for common AI-generated presentation and slide selector flags while preserving the canonical interface.Changes
--presentation-idalias for--presentation--slidealias:p[A-Za-z]{2,}routes to slide ID--slide-ids--slide-numbers--slides--slidevalues with typed validation metadataTest Plan
GOTOOLCHAIN=go1.23.12 make buildGOTOOLCHAIN=go1.23.12 make unit-testGOTOOLCHAIN=go1.23.12 go vet ./...gofmt -l .produces no outputGOTOOLCHAIN=go1.23.12 go mod tidyleavesgo.modandgo.sumunchangedgolangci-lintreports 0 issuesRelated Issues
Summary by CodeRabbit
New Features
--presentation-id.Bug Fixes