Skip to content

feat(sheets): add float image shortcuts#494

Merged
fangshuyu-768 merged 1 commit intomainfrom
feat/sheet_float
Apr 18, 2026
Merged

feat(sheets): add float image shortcuts#494
fangshuyu-768 merged 1 commit intomainfrom
feat/sheet_float

Conversation

@caojie0621
Copy link
Copy Markdown
Collaborator

@caojie0621 caojie0621 commented Apr 15, 2026

Summary

Add a complete set of shortcuts for floating images on spreadsheets, plus a
dedicated media upload shortcut that correctly handles both the single-part
and multipart upload paths for parent_type=sheet_image.

New shortcuts

Shortcut Purpose
sheets +media-upload Upload a local image as parent_type=sheet_image; auto-switches to multipart for > 20 MB. Returns file_token for use as --float-image-token.
sheets +create-float-image Create a floating image anchored at a single cell.
sheets +update-float-image Update range / width / height / offset. At least one mutable field required.
sheets +get-float-image Fetch one float image's metadata.
sheets +list-float-images List all float images on a sheet.
sheets +delete-float-image Delete a float image by id.

Client-side validation (create / update)

  • --range must be a single anchor cell (sheetId!A1:A1); multi-cell spans rejected.
  • --range prefix must equal --sheet-id (catches --sheet-id sheet1 --range other!A1:A1 locally).
  • --width / --height must be >= 20 pixels when set.
  • --offset-x / --offset-y must be >= 0 when set. Upper bound (< anchor cell's width/height) is enforced server-side and surfaced via a friendly hint (see below).
  • +update-float-image requires at least one of --range / --width / --height / --offset-x / --offset-y; ID-only invocations are rejected before a PATCH {} can reach the
    server.

Error classifier

Add LarkErrSheetsFloatImageInvalidDims = 1310246 to the centralized
classifier. When the backend rejects width / height / offset values, the CLI
surfaces an invalid_params error with a hint pointing at the four flags.

Flag typing

Width / height / offset-x / offset-y flags are declared Type: "int" and
"width":"300". Aligns with other sheets shortcuts that take numeric flags.

Scope least-privilege

Create / update / delete shortcuts declare only sheets:spreadsheet:write_only
(dropping the previously declared sheets:spreadsheet:read), matching the
SKILL.md permission table.

cmd/api form-field serialization bug fix

internal/cmdutil/fileupload.go#BuildFormdata previously rendered form field
values with fmt.Sprintf("%v", v). JSON numbers unmarshal to float64, and
%v delegates to %g which switches to scientific notation at ~1e6
(e.g. 1185356"1.185356e+06"). Backends parsing the field as an integer
then reject it (seen as 1061002 params error when uploading a 1.16 MB image
via lark-cli api --file ... --data '{"size":1185356}').

Introduce formatFormFieldValue that uses
strconv.FormatFloat(n, 'f', -1, 64) for float64 and falls through to %v
for other types. Fixes all raw-API uploads carrying large numeric fields,
not just the float-image case. Shortcut paths that use
common.UploadDriveMedia{All,Multipart} were already safe because those
helpers format integers via %d explicitly.

Test plan

  • go test ./shortcuts/sheets/ ./internal/cmdutil/ ./cmd/api/ ./cmd/service/ ./internal/output/
  • Create / update / delete / list / get on a live spreadsheet (21 scenarios incl. boundaries: --width 20, --offset-x 0, multi-cell range rejection, sheet-id prefix mismatch,
    negative offsets, empty update payload, server-side 1310246 with hint, bot vs user read permissions, bot docs +media-preview access to the returned token).
  • Unit: TestFormatFormFieldValue table-drives the scientific-notation boundary and pass-through types.
  • Unit: float-image validators (single-cell, sheet-id match, dim bounds, empty payload).
  • Unit: normalizeBatchStyleRanges was addressed on a sibling branch and is out of scope here.

Skill docs

  • New lark-sheets-media-upload.md.
  • New lark-sheets-{create,update,get,list,delete}-float-image.md.
  • SKILL.md adds a Float Images table plus a pointer to docs +media-preview
    for reading image bytes (the read APIs only return metadata + token).

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds five Sheets float-image CLI shortcuts (create, update, get, list, delete), implements path/token helpers and validations (range/sheet-id/dims), conditionally builds request bodies, adds end-to-end tests and docs, and maps a new Lark error code for invalid float-image dimensions.

Changes

Cohort / File(s) Summary
Float Image Shortcuts Implementation
shortcuts/sheets/sheet_float_image.go
New file adding five public shortcuts and helpers: URL-encoded path builders, spreadsheet-token resolution (from --spreadsheet-token or parsed --url), single-cell --range / --sheet-id validation, numeric bounds checks (applied only to explicitly changed flags), and request-body assembly that includes only changed fields and conditionally the float_image_token.
Tests
shortcuts/sheets/sheet_float_image_test.go
New E2E-style tests covering Validate, DryRun (method/path/query/body), and Execute flows for create/update/get/list/delete; asserts validation failures, bounds checks, sheet-id/range consistency, and correct HTTP method/path and JSON payload contents; includes mocked API error handling.
Shortcuts Registry
shortcuts/sheets/shortcuts.go
Appends the five new shortcut variables (SheetCreateFloatImage, SheetUpdateFloatImage, SheetGetFloatImage, SheetListFloatImages, SheetDeleteFloatImage) to the returned slice from Shortcuts().
Skill & Reference Docs
skills/lark-sheets/SKILL.md, skills/lark-sheets/references/...
Adds "floating image" capability entry and five reference docs with usage, examples, token acquisition guidance, parameter constraints (--range, --width/--height >=20, --offset-* >=0), --dry-run behavior, output shapes (meta-only, use float_image_token + docs +media-preview to fetch bytes), and permission mappings.
Error Mapping & Tests
internal/output/lark_errors.go, internal/output/lark_errors_test.go
Adds exported constant LarkErrSheetsFloatImageInvalidDims = 1310246 and maps it in ClassifyLarkError to (ExitAPI, "invalid_params", hint) referencing --width / --height / --offset-x / --offset-y; adds unit test asserting the mapping and hint content.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as Client (lark-cli)
    participant RT as Runtime (shortcut runtime)
    participant API as Sheets API
    rect rgba(200,230,255,0.5)
    CLI->>RT: Parse flags (--url / --spreadsheet-token, --sheet-id, --range, float-image flags...)
    RT->>RT: resolveSpreadsheetToken(from --spreadsheet-token or parsed --url)
    RT->>RT: validateRangeAndSheetID()
    RT->>RT: validateFloatImageDims(if flags changed)
    RT->>RT: buildFloatImagePathAndBody(include changed fields, maybe float_image_token)
    CLI->>API: HTTP POST/PATCH/GET/DELETE /spreadsheets/{token}/sheets/{sheet}/float_images...
    API-->>CLI: JSON response (float_image / items / code/msg)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • fangshuyu-768
  • jackie3927

Poem

🐰 I hopped through flags and URL vines,
I snuck your token from parsed lines,
I checked the range and kept it small,
Built body fields, sent calls one and all,
Tests and docs—carrots for dev times. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.03% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding five float image shortcuts to the sheets module.
Description check ✅ Passed The PR description follows the required template with all key sections present: Summary, Changes, Test Plan, and Related Issues. It comprehensively documents the feature additions, validation rules, error handling, and testing approach.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/sheet_float

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels Apr 15, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
shortcuts/sheets/sheet_float_image_test.go (1)

131-340: 建议补充 Update/List/Delete 的 Validate 失败用例。

当前负向校验主要覆盖了 create/get 的 token 缺失;建议为 update/list/delete 也补齐缺少 --sheet-id / --float-image-id / token 的失败断言,避免参数校验后续回退。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_float_image_test.go` around lines 131 - 340, Add
negative Validate unit tests for update/list/delete float-image commands similar
to TestGetFloatImageValidateMissingToken: call SheetUpdateFloatImage.Validate,
SheetListFloatImages.Validate, and SheetDeleteFloatImage.Validate with runtimes
missing required flags (e.g. missing "--spreadsheet-token" or "--url", missing
"--sheet-id", and for update/delete missing "--float-image-id") and assert the
returned error is non-nil and contains the proper hint (e.g. "--url or
--spreadsheet-token" or "--sheet-id" or "--float-image-id"); place tests named
like TestUpdateFloatImageValidateMissingToken,
TestUpdateFloatImageValidateMissingSheetID,
TestUpdateFloatImageValidateMissingFloatImageID, and analogous ones for List
(missing token/sheet-id) and Delete (missing token/sheet-id/float-image-id) so
validation coverage matches create/get.
skills/lark-sheets/references/lark-sheets-update-float-image.md (1)

23-34: 建议明确“至少提供一个可更新字段”的行为约束。

当前参数表里可变字段都标为可选,建议补一句:未提供 --range/--width/--height/--offset-x/--offset-y 时的实际行为(报错或无操作),避免用户误用。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-sheets/references/lark-sheets-update-float-image.md` around lines
23 - 34, The parameter table leaves all updateable flags optional but doesn't
state the required "at least one" constraint; update the
lark-sheets-update-float-image.md doc to explicitly require that at least one of
--range, --width, --height, --offset-x, or --offset-y must be provided when
calling the command and state the CLI behavior when none are supplied (e.g.,
fail with a clear error such as "At least one of --range, --width, --height,
--offset-x, --offset-y must be provided" and exit non‑zero), referencing those
exact flag names so users know the enforced rule and expected error handling.
shortcuts/sheets/sheet_float_image.go (2)

94-95: Don’t ignore token-validation errors in Execute paths.

At Line 94, Line 139, Line 175, Line 209, and Line 244, token, _ := validateFloatImageToken(runtime) discards errors. Even with Validate present, explicit error handling keeps Execute robust against future call-path changes.

Suggested fix pattern (apply to all five Execute handlers)
-		token, _ := validateFloatImageToken(runtime)
+		token, err := validateFloatImageToken(runtime)
+		if err != nil {
+			return err
+		}
 		body := buildFloatImageBody(runtime, true)
 		if s := runtime.Str("float-image-id"); s != "" {
 			body["float_image_id"] = s
 		}
-		data, err := runtime.CallAPI("POST", floatImageBasePath(token, runtime.Str("sheet-id")), nil, body)
+		data, err := runtime.CallAPI("POST", floatImageBasePath(token, runtime.Str("sheet-id")), nil, body)
 		if err != nil {
 			return err
 		}

Also applies to: 139-140, 175-176, 209-210, 244-245

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_float_image.go` around lines 94 - 95, The Execute
handlers are discarding errors from validateFloatImageToken by using token, _ :=
validateFloatImageToken(runtime); update each Execute path that calls
validateFloatImageToken (the five handlers that then call buildFloatImageBody)
to capture and handle the error (e.g., token, err :=
validateFloatImageToken(runtime); if err != nil { return nil, err } or propagate
a wrapped error) before using token; ensure you apply the same explicit error
check in every handler that currently ignores the error to keep Execute robust.

127-130: Reject no-op update requests early.

+update-float-image currently succeeds validation even when no mutable field is provided, which can produce an empty PATCH body and a server-side error. Add a client-side check for at least one of --range, --width, --height, --offset-x, --offset-y.

Suggested fix
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
-		_, err := validateFloatImageToken(runtime)
-		return err
+		if _, err := validateFloatImageToken(runtime); err != nil {
+			return err
+		}
+		if !runtime.Cmd.Flags().Changed("range") &&
+			!runtime.Cmd.Flags().Changed("width") &&
+			!runtime.Cmd.Flags().Changed("height") &&
+			!runtime.Cmd.Flags().Changed("offset-x") &&
+			!runtime.Cmd.Flags().Changed("offset-y") {
+			return common.FlagErrorf("specify at least one field to update: --range, --width, --height, --offset-x, or --offset-y")
+		}
+		return nil
 	},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_float_image.go` around lines 127 - 130, The validator
for the update-float-image command currently only calls
validateFloatImageToken(runtime) and allows no-op updates; change the Validate
closure to first obtain the token (call validateFloatImageToken(runtime) and
capture its result) and then check that at least one mutable field is present
(one of the flags/fields: range, width, height, offset-x, offset-y). If none are
provided, return a validation error indicating that at least one of --range,
--width, --height, --offset-x, or --offset-y must be set; otherwise proceed to
return nil (or the original err). Apply this change inside the Validate function
in sheet_float_image.go where validateFloatImageToken is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@shortcuts/sheets/sheet_float_image.go`:
- Around line 25-27: The code currently assigns token =
extractSpreadsheetToken(runtime.Str("url")) whenever runtime.Str("url") is
non-empty, which can clear a previously valid token; change the logic in the
block that checks runtime.Str("url") so you call extractSpreadsheetToken and
only overwrite token if the parsed result is non-empty (e.g., parsed :=
extractSpreadsheetToken(runtime.Str("url")); if parsed != "" { token = parsed
}), ensuring an already-provided runtime.Str("spreadsheet-token") is preserved
when URL parsing fails.

In `@skills/lark-sheets/references/lark-sheets-create-float-image.md`:
- Around line 19-22: The example upload command uses a non-executable
placeholder for the JSON "size" field which will break copy-paste; update the
example in the lark-cli POST example (the line starting with "lark-cli api POST
/open-apis/drive/v1/medias/upload_all") to show a valid JSON value for "size"
(either a concrete integer like 12345 or a shell variable notation such as
"$FILE_SIZE") instead of "<文件字节数>", and ensure the surrounding JSON remains
valid so the command can be copied and executed as-is.

In `@skills/lark-sheets/SKILL.md`:
- Around line 224-232: The new "浮动图片" shortcuts (`+create-float-image`,
`+update-float-image`, `+get-float-image`, `+list-float-images`,
`+delete-float-image`) were added but the corresponding entries are missing from
the later "API Resources" / "权限表" sections; update the document to add matching
resource and permission mappings for float image operations
(create/update/get/list/delete) so scopes and resource names align with those
shortcut names, listing required API resource identifiers and required OAuth
scopes/permissions next to each shortcut to prevent permission mismatches.

---

Nitpick comments:
In `@shortcuts/sheets/sheet_float_image_test.go`:
- Around line 131-340: Add negative Validate unit tests for update/list/delete
float-image commands similar to TestGetFloatImageValidateMissingToken: call
SheetUpdateFloatImage.Validate, SheetListFloatImages.Validate, and
SheetDeleteFloatImage.Validate with runtimes missing required flags (e.g.
missing "--spreadsheet-token" or "--url", missing "--sheet-id", and for
update/delete missing "--float-image-id") and assert the returned error is
non-nil and contains the proper hint (e.g. "--url or --spreadsheet-token" or
"--sheet-id" or "--float-image-id"); place tests named like
TestUpdateFloatImageValidateMissingToken,
TestUpdateFloatImageValidateMissingSheetID,
TestUpdateFloatImageValidateMissingFloatImageID, and analogous ones for List
(missing token/sheet-id) and Delete (missing token/sheet-id/float-image-id) so
validation coverage matches create/get.

In `@shortcuts/sheets/sheet_float_image.go`:
- Around line 94-95: The Execute handlers are discarding errors from
validateFloatImageToken by using token, _ := validateFloatImageToken(runtime);
update each Execute path that calls validateFloatImageToken (the five handlers
that then call buildFloatImageBody) to capture and handle the error (e.g.,
token, err := validateFloatImageToken(runtime); if err != nil { return nil, err
} or propagate a wrapped error) before using token; ensure you apply the same
explicit error check in every handler that currently ignores the error to keep
Execute robust.
- Around line 127-130: The validator for the update-float-image command
currently only calls validateFloatImageToken(runtime) and allows no-op updates;
change the Validate closure to first obtain the token (call
validateFloatImageToken(runtime) and capture its result) and then check that at
least one mutable field is present (one of the flags/fields: range, width,
height, offset-x, offset-y). If none are provided, return a validation error
indicating that at least one of --range, --width, --height, --offset-x, or
--offset-y must be set; otherwise proceed to return nil (or the original err).
Apply this change inside the Validate function in sheet_float_image.go where
validateFloatImageToken is invoked.

In `@skills/lark-sheets/references/lark-sheets-update-float-image.md`:
- Around line 23-34: The parameter table leaves all updateable flags optional
but doesn't state the required "at least one" constraint; update the
lark-sheets-update-float-image.md doc to explicitly require that at least one of
--range, --width, --height, --offset-x, or --offset-y must be provided when
calling the command and state the CLI behavior when none are supplied (e.g.,
fail with a clear error such as "At least one of --range, --width, --height,
--offset-x, --offset-y must be provided" and exit non‑zero), referencing those
exact flag names so users know the enforced rule and expected error handling.
🪄 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: ad85d713-c329-44bc-a554-e29c863d4a0a

📥 Commits

Reviewing files that changed from the base of the PR and between 03ba542 and c01f3ec.

📒 Files selected for processing (9)
  • shortcuts/sheets/sheet_float_image.go
  • shortcuts/sheets/sheet_float_image_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md

Comment thread shortcuts/sheets/sheet_float_image.go Outdated
Comment thread skills/lark-sheets/references/lark-sheets-create-float-image.md Outdated
Comment thread skills/lark-sheets/SKILL.md
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ff6ee98ca58a2bf2fc72035a3d1592a3037c70c7

🧩 Skill update

npx skills add larksuite/cli#feat/sheet_float -y -g

Comment thread shortcuts/sheets/sheet_float_image.go Outdated
Comment thread shortcuts/sheets/sheet_float_image.go
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
shortcuts/sheets/sheet_float_image.go (1)

23-33: ⚠️ Potential issue | 🟠 Major

Preserve --spreadsheet-token when --url cannot be parsed.

Line 26 still allows extractSpreadsheetToken(u) returning "" to clear a valid explicit token because "" != u. Only overwrite when parsing produced a non-empty token; otherwise fail only if no fallback token exists.

Suggested fix
 func validateFloatImageToken(runtime *common.RuntimeContext) (string, error) {
 	token := runtime.Str("spreadsheet-token")
 	if u := runtime.Str("url"); u != "" {
-		if parsed := extractSpreadsheetToken(u); parsed != u {
+		if parsed := extractSpreadsheetToken(u); parsed != "" {
 			token = parsed
+		} else if token == "" {
+			return "", common.FlagErrorf("invalid --url; specify a valid spreadsheet URL or --spreadsheet-token")
 		}
 	}
 	if token == "" {
 		return "", common.FlagErrorf("specify --url or --spreadsheet-token")
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_float_image.go` around lines 23 - 33, In
validateFloatImageToken, avoid overwriting an explicit
runtime.Str("spreadsheet-token") with an empty result from
extractSpreadsheetToken(runtime.Str("url")); change the condition that currently
checks parsed != u to only accept a non-empty parsed token (e.g., if parsed :=
extractSpreadsheetToken(u); parsed != "" { token = parsed }), so we only replace
token when parsing produced a real token and otherwise keep the explicit
--spreadsheet-token as fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@shortcuts/sheets/sheet_float_image.go`:
- Line 80: The Scopes slices in the shortcut declarations (the Scopes field in
the struct literals) in sheet_float_image.go include "sheets:spreadsheet:read"
even though these shortcuts perform only create/patch/delete operations; remove
"sheets:spreadsheet:read" from those Scopes slices so they only request
"sheets:spreadsheet:write_only". Update each Scopes occurrence (the Scopes field
around the existing entries at the three shortcut definitions) to drop the read
scope, preserving the rest of the struct and formatting.
- Around line 88-98: The Validate function in sheet_float_image.go currently
calls validateFloatImageToken(...) and validateFloatImageRange(...), but does
not check client-side numeric bounds for flags "width", "height" (must be >=20)
and "offset-x", "offset-y" (must be >=0) before sending requests; add explicit
validation in Validate after validateFloatImageToken and before
validateFloatImageRange to parse runtime.Int("width"), runtime.Int("height"),
runtime.Int("offset-x"), runtime.Int("offset-y") (or runtime.Str->Atoi if
applicable) and return descriptive errors when values are out of bounds, and add
unit tests covering each invalid case (width<20, height<20, offset-x<0,
offset-y<0) to assert Validate fails locally; keep existing
validateFloatImageToken and validateFloatImageRange calls intact.

---

Duplicate comments:
In `@shortcuts/sheets/sheet_float_image.go`:
- Around line 23-33: In validateFloatImageToken, avoid overwriting an explicit
runtime.Str("spreadsheet-token") with an empty result from
extractSpreadsheetToken(runtime.Str("url")); change the condition that currently
checks parsed != u to only accept a non-empty parsed token (e.g., if parsed :=
extractSpreadsheetToken(u); parsed != "" { token = parsed }), so we only replace
token when parsing produced a real token and otherwise keep the explicit
--spreadsheet-token as fallback.
🪄 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: 9e18c838-2e81-4a01-9a0b-d56e679d2bf5

📥 Commits

Reviewing files that changed from the base of the PR and between db0e69a and 34ecf55.

📒 Files selected for processing (11)
  • internal/output/lark_errors.go
  • internal/output/lark_errors_test.go
  • shortcuts/sheets/sheet_float_image.go
  • shortcuts/sheets/sheet_float_image_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
✅ Files skipped from review due to trivial changes (6)
  • internal/output/lark_errors_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/sheets/sheet_float_image_test.go

Comment thread shortcuts/sheets/sheet_float_image.go Outdated
Comment thread shortcuts/sheets/sheet_float_image.go Outdated
Comment thread shortcuts/sheets/sheet_float_image.go
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
shortcuts/sheets/sheet_float_image_test.go (1)

149-163: Consider exercising the new 1310246 classification through the shortcut.

TestCreateFloatImageExecuteAPIError uses a generic code: 90001 to assert error propagation. Since this PR specifically introduces LarkErrSheetsFloatImageInvalidDims (1310246) with a dedicated hint, adding one scenario that returns code: 1310246 and asserts the resulting error/hint surface (e.g., mentions --width / --height) would give end-to-end confidence that the classification reaches users through this shortcut — not just the unit test in internal/output/lark_errors_test.go.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@shortcuts/sheets/sheet_float_image_test.go` around lines 149 - 163, Update
the TestCreateFloatImageExecuteAPIError test (and/or add a new test) to exercise
the new LarkErrSheetsFloatImageInvalidDims classification by registering an
httpmock response with code 1310246 for the POST to
/open-apis/sheets/v3/spreadsheets/shtTOKEN/sheets/sheet1/float_images, invoke
mountAndRunSheets with SheetCreateFloatImage as done now, and assert the
returned error includes the classification/hint text (e.g., mentions "--width"
or "--height") so the shortcut surfaces the specific hint to users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@shortcuts/sheets/sheet_float_image_test.go`:
- Around line 149-163: Update the TestCreateFloatImageExecuteAPIError test
(and/or add a new test) to exercise the new LarkErrSheetsFloatImageInvalidDims
classification by registering an httpmock response with code 1310246 for the
POST to /open-apis/sheets/v3/spreadsheets/shtTOKEN/sheets/sheet1/float_images,
invoke mountAndRunSheets with SheetCreateFloatImage as done now, and assert the
returned error includes the classification/hint text (e.g., mentions "--width"
or "--height") so the shortcut surfaces the specific hint to users.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 993a5667-448c-40b3-bb6f-cc756203649a

📥 Commits

Reviewing files that changed from the base of the PR and between 34ecf55 and f8a122a.

📒 Files selected for processing (11)
  • internal/output/lark_errors.go
  • internal/output/lark_errors_test.go
  • shortcuts/sheets/sheet_float_image.go
  • shortcuts/sheets/sheet_float_image_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
✅ Files skipped from review due to trivial changes (6)
  • internal/output/lark_errors_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/sheets/sheet_float_image.go

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
skills/lark-sheets/references/lark-sheets-create-float-image.md (1)

17-37: Minor: the three SIZE= lines will silently shadow each other if copy-pasted as a block.

Users copying the whole block will run all three stat/wc variants sequentially; on Linux the BSD stat -f%z call will print an error to stderr and the GNU line will then overwrite SIZE, so it still "works" but is noisy and confusing. Consider making them clearly alternative (e.g., wrap each in its own fenced block, or prefix with # choose one: and comment out two of them by default).

📝 Optional tweak
-#    macOS / BSD:
-SIZE=$(stat -f%z ./image.png)
-#    Linux / GNU:
-SIZE=$(stat -c%s ./image.png)
-#    POSIX shell / Git-Bash on Windows:
-SIZE=$(wc -c < ./image.png | tr -d ' ')
+# Choose ONE of the following (uncomment the line matching your platform):
+# macOS / BSD:
+# SIZE=$(stat -f%z ./image.png)
+# Linux / GNU:
+# SIZE=$(stat -c%s ./image.png)
+# POSIX shell / Git-Bash on Windows (portable):
+SIZE=$(wc -c < ./image.png | tr -d ' ')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@skills/lark-sheets/references/lark-sheets-create-float-image.md` around lines
17 - 37, The three alternative SIZE assignment lines (the `SIZE=$(stat -f%z
./image.png)`, `SIZE=$(stat -c%s ./image.png)`, `SIZE=$(wc -c < ./image.png | tr
-d ' ')`) will run sequentially if copy-pasted, causing noise and shadowing;
update the snippet around the SIZE variable so alternatives are explicit
choices—either present each platform variant in its own fenced block or comment
out the two unused lines and add a “# choose one” note, or implement a small
OS-detection wrapper that sets SIZE once (refer to the `SIZE` variable and the
three stat/wc command variants) before the `lark-cli` upload step and ensure
only one assignment executes when users copy the block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@skills/lark-sheets/references/lark-sheets-create-float-image.md`:
- Around line 17-37: The three alternative SIZE assignment lines (the
`SIZE=$(stat -f%z ./image.png)`, `SIZE=$(stat -c%s ./image.png)`, `SIZE=$(wc -c
< ./image.png | tr -d ' ')`) will run sequentially if copy-pasted, causing noise
and shadowing; update the snippet around the SIZE variable so alternatives are
explicit choices—either present each platform variant in its own fenced block or
comment out the two unused lines and add a “# choose one” note, or implement a
small OS-detection wrapper that sets SIZE once (refer to the `SIZE` variable and
the three stat/wc command variants) before the `lark-cli` upload step and ensure
only one assignment executes when users copy the block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10f4d439-d91c-47e0-9a05-f1ef4d28cb78

📥 Commits

Reviewing files that changed from the base of the PR and between f8a122a and 95b3812.

📒 Files selected for processing (11)
  • internal/output/lark_errors.go
  • internal/output/lark_errors_test.go
  • shortcuts/sheets/sheet_float_image.go
  • shortcuts/sheets/sheet_float_image_test.go
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/SKILL.md
  • skills/lark-sheets/references/lark-sheets-create-float-image.md
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
✅ Files skipped from review due to trivial changes (8)
  • internal/output/lark_errors_test.go
  • skills/lark-sheets/references/lark-sheets-delete-float-image.md
  • skills/lark-sheets/references/lark-sheets-get-float-image.md
  • shortcuts/sheets/shortcuts.go
  • skills/lark-sheets/references/lark-sheets-update-float-image.md
  • skills/lark-sheets/references/lark-sheets-list-float-images.md
  • shortcuts/sheets/sheet_float_image_test.go
  • shortcuts/sheets/sheet_float_image.go

@caojie0621 caojie0621 force-pushed the feat/sheet_float branch 4 times, most recently from a893bed to f9145ee Compare April 18, 2026 09:42
@larksuite larksuite deleted a comment from codecov Bot Apr 18, 2026
Implement +create-float-image, +update-float-image, +get-float-image,
+list-float-images, and +delete-float-image shortcuts wrapping the v3
spreadsheet float_image API. The create reference doc includes the
prerequisite media upload step with the correct parent_type
(sheet_image) to avoid common token mismatch errors.
@larksuite larksuite deleted a comment from codecov Bot Apr 18, 2026
@fangshuyu-768 fangshuyu-768 merged commit abb02cd into main Apr 18, 2026
21 checks passed
@fangshuyu-768 fangshuyu-768 deleted the feat/sheet_float branch April 18, 2026 15:27
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 84.87085% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.11%. Comparing base (5a0e1d3) to head (ff6ee98).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/sheets/sheet_media_upload.go 80.61% 13 Missing and 6 partials ⚠️
shortcuts/sheets/sheet_float_image.go 89.87% 8 Missing and 8 partials ⚠️
shortcuts/sheets/shortcuts.go 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #494      +/-   ##
==========================================
+ Coverage   59.91%   60.11%   +0.20%     
==========================================
  Files         388      390       +2     
  Lines       33147    33417     +270     
==========================================
+ Hits        19859    20090     +231     
- Misses      11420    11446      +26     
- Partials     1868     1881      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants