feat(sheets): add undo shortcut - #1715
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a registered ChangesSheets undo shortcut
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant Undo
participant undo_last
Runtime->>Undo: invoke +undo with spreadsheet locator and count
Undo->>Undo: validate count and resolve spreadsheet token
Undo->>undo_last: send excel_id and count
undo_last-->>Runtime: return undo result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
572c324 to
d48bf26
Compare
9f93cce to
74d65e6
Compare
5d9cb51 to
31a8b47
Compare
d48bf26 to
42870f0
Compare
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/sheets/lark_sheet_undo_test.go`:
- Around line 77-79: Replace the substring-only assertion in the relevant
error-path test with structured checks using errs.ProblemOf: require a typed
problem with errs.SubtypeInvalidArgument, then use errors.As to require
*errs.ValidationError and validate its Param against the expected count
parameter. Preserve the existing precise-message check only if needed, and
verify the wrapped cause as required by the test guideline; add imports for
errors and errs if missing.
🪄 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
Run ID: 69cc976a-a77a-404a-a3e6-8dc3c153e479
📒 Files selected for processing (7)
shortcuts/sheets/data/flag-defs.jsonshortcuts/sheets/flag_defs_gen.goshortcuts/sheets/lark_sheet_undo.goshortcuts/sheets/lark_sheet_undo_test.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-history.md
| if !strings.Contains(err.Error(), tt.wantSub) { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert typed metadata for error paths.
The test uses strings.Contains to assert the error, which violates the **/*_test.go coding guideline: "Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone."
Please update the assertion to verify the typed Subtype (e.g., errs.SubtypeInvalidArgument) and the Param (using errors.As(err, &ve) for validation errors). As per coding guidelines, Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone.
🛠️ Proposed fix
Replace the substring check with a structured assertion:
p, ok := errs.ProblemOf(err)
if !ok {
t.Fatalf("expected typed error, got: %v", err)
}
if p.Subtype != errs.SubtypeInvalidArgument {
t.Errorf("Subtype = %v, want %v", p.Subtype, errs.SubtypeInvalidArgument)
}
var ve *errs.ValidationError
if errors.As(err, &ve) {
if ve.Param != "count" && ve.Param != "--count" {
t.Errorf("Param = %q, want count or --count", ve.Param)
}
} else {
t.Errorf("expected *errs.ValidationError")
}
// Keep the substring assertion to ensure the precise guidance message matches.
if !strings.Contains(err.Error(), tt.wantSub) {
t.Fatalf("unexpected error: %v", err)
}(Note: Ensure that "errors" and "github.com/larksuite/cli/internal/errs" are added to the file's imports if not already present.)
🤖 Prompt for 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.
In `@shortcuts/sheets/lark_sheet_undo_test.go` around lines 77 - 79, Replace the
substring-only assertion in the relevant error-path test with structured checks
using errs.ProblemOf: require a typed problem with errs.SubtypeInvalidArgument,
then use errors.As to require *errs.ValidationError and validate its Param
against the expected count parameter. Preserve the existing precise-message
check only if needed, and verify the wrapped cause as required by the test
guideline; add imports for errors and errs if missing.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1715 +/- ##
=======================================
Coverage 74.66% 74.66%
=======================================
Files 877 878 +1
Lines 91731 91755 +24
=======================================
+ Hits 68494 68512 +18
- Misses 17926 17929 +3
- Partials 5311 5314 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@10363e08d1b2c709238fb618e621de7ad0e7adc2🧩 Skill updatenpx skills add larksuite/cli#feat/sheets-undo-shortcut-zzj -y -g |
88c22ff to
5d1a5ee
Compare
5d1a5ee to
10363e0
Compare
Summary
lark-cli sheets +undoas a user-only shortcut.undo_lastwrite tool withexcel_idas the only input.Verification
go test -count=1 ./shortcuts/sheets/..../build.shsheets +undo, verify A1 is cleared, then runsheets +undoagain and verify the empty-stack response.Summary by CodeRabbit
+undoshortcut to undo the calling user’s latest AI-generated spreadsheet write(s).--count(1–20) for multi-step undo.--yes; supports--dry-runto preview the operation.+undobehavior, limits, and how it differs from version rollback.+undo, including dry-run, request payload verification,--countvalidation, confirmation gating, and output mapping.