Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix(sheets): add mention_type enum to set_cell_range cells schema#1439

Open
zhengzhijiej-tech wants to merge 1 commit into
main from
fix/sheet-mention-type-enum
Open

fix(sheets): add mention_type enum to set_cell_range cells schema #1439
zhengzhijiej-tech wants to merge 1 commit into
main from
fix/sheet-mention-type-enum

Conversation

@zhengzhijiej-tech

@zhengzhijiej-tech zhengzhijiej-tech commented Jun 12, 2026
edited
Loading

Copy link
Copy Markdown
Collaborator

Problem

A set_cell_range rich_text @mention with an unsupported mention_type (e.g. 6 = cloud shared folder) was copied verbatim into the setRangeValues mutation's mentionFileInfo.fileType and failed pb serialization (mentionFileInfo.fileType: enum value expected), taking the whole write down. The --cells schema declared mention_type as a bare number with no enum.

Change

  • shortcuts/sheets/data/flag-schemas.json: mention_type now carries an enum — the render-side MENTION_FILE_TYPE set [0,1,3,8,11,12,15,16,22,30,38] (the set checkIsEmbed / MENTION_TO_SUITE recognize), plus a per-value description. Values that are proto-serializable but absent from this set (FOLDER=2, SHEET_DOC=4, CHAT=5) would survive serialization yet not render — a silent broken mention — so they are excluded too. 0 / omitted = a user @mention. Synced from the upstream tool schema.
  • shortcuts/sheets/lark_sheet_write_cells_test.go: reject 6, allow 0 / 22.

Verification

go test ./shortcuts/sheets/ passes.

coderabbitai Bot commented Jun 12, 2026
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Restricts rich_text[].mention.mention_type to an explicit numeric enum and adds tests that assert rejected unsupported values and accepted valid enum values during cell-set validation.

Changes

Mention Type Enum Validation

Layer / File(s) Summary
Schema enum constraint
shortcuts/sheets/data/flag-schemas.json
mention_type in rich_textmention is constrained from an unconstrained number to explicit enum [0, 1, 3, 8, 11, 12, 15, 16, 22, 30, 38].
Validation tests
shortcuts/sheets/lark_sheet_write_cells_test.go
Adds fmt import and two tests: one ensures an out-of-enum mention_type is rejected, the other verifies allowed mention types (e.g., 0, 22) pass validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size/M

Suggested reviewers

  • fangshuyu-768

Poem

🐇 I nibbled code beneath the moonlight glow,
I hopped through enums, tidy row by row.
No wild numbers left to stray or roam,
Each mention now has a cozy home.
Hooray for schemas, neat and bright — hop on!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: adding an enum constraint to the mention_type field in the sheets schema.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description comprehensively covers the problem, solution, and verification with good technical detail aligned to the template structure.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sheet-mention-type-enum

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/M Single-domain feat or fix with limited business impact labels Jun 12, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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_write_cells_test.go`:
- Around line 447-450: Replace the brittle substring-based assertion around
combined := stdout + stderr + err.Error() with typed error assertions: use
errs.ProblemOf(err) to extract problem metadata and assert problem.Category and
problem.Subtype match the expected values (e.g., mention_type / not-in-enum),
and use errors.As(err, new(*errs.ValidationError)) to assert the Param is the
expected field and that the underlying cause is preserved (compare
ProblemOf().Cause or check errors.Is against the original cause). Update the
test around the strings.Contains checks to perform these typed assertions
instead of substring matching.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

i️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01e3a37b-813d-4abe-9eee-ea45d6106e1b

📥 Commits

Reviewing files that changed from the base of the PR and between 8e60f01 and ab35368.

📒 Files selected for processing (2)
  • shortcuts/sheets/data/flag-schemas.json
  • shortcuts/sheets/lark_sheet_write_cells_test.go

Comment on lines +447 to +450
combined := stdout + stderr + err.Error()
if !strings.Contains(combined, "mention_type") || !strings.Contains(combined, "not in enum") {
t.Errorf("expected mention_type enum guard; got=%s|%s|%v", stdout, stderr, err)
}

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use typed error assertions for this error-path test.

This check is message-substring based; for error-path tests here, assert typed metadata (category/subtype) and cause preservation instead. For param, use errors.As(..., *errs.ValidationError) rather than errs.ProblemOf.

Based on learnings and coding guidelines: *_test.go error paths should use errs.ProblemOf + cause checks, and Param must be asserted via errors.As on *errs.ValidationError.

🤖 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_write_cells_test.go` around lines 447 - 450,
Replace the brittle substring-based assertion around combined := stdout + stderr
+ err.Error() with typed error assertions: use errs.ProblemOf(err) to extract
problem metadata and assert problem.Category and problem.Subtype match the
expected values (e.g., mention_type / not-in-enum), and use errors.As(err,
new(*errs.ValidationError)) to assert the Param is the expected field and that
the underlying cause is preserved (compare ProblemOf().Cause or check errors.Is
against the original cause). Update the test around the strings.Contains checks
to perform these typed assertions instead of substring matching.

Sources: Coding guidelines, Learnings

codecov Bot commented Jun 12, 2026
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.87%. Comparing base (2a7e9c7) to head (4227973).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@
## main #1439 +/- ##
==========================================
+ Coverage 72.83% 72.87% +0.04% 
==========================================
 Files 732 737 +5 
 Lines 69140 69446 +306 
==========================================
+ Hits 50356 50609 +253 
- Misses 15003 15037 +34 
- Partials 3781 3800 +19 

☔ View full report in Codecov by Harness.
📢 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.

github-actions Bot commented Jun 12, 2026
edited
Loading

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/sheet-mention-type-enum -y -g

Constrain rich_text mention_type to the proto MENTION_FILE_TYPE set so a
file @mention with an out-of-enum value (e.g. 6 = cloud shared folder) is
rejected by the schema validator before it reaches the server and fails
pb serialization ("mentionFileInfo.fileType: enum value expected").
- data/flag-schemas.json: mention_type gains enum + per-value description
- lark_sheet_write_cells_test.go: cover reject (6) + allow (0 / 2 / 22)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/data/flag-schemas.json`:
- Around line 465-479: The enum for the mention type is missing values required
by the proto (add 2, 4, and 5); update the "enum" array tied to the property
whose "description" begins with "`@提及类型编号`(仅 type='mention' 时可选)" so it includes
0,1,2,3,4,5,8,11,12,15,16,22,30,38, ensuring the schema's allowed mention_type
values match the MENTION_FILE_TYPE allowlist and will not reject valid mentions
at parse time.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

i️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: adcdddd7-e99e-4da4-b0c3-b6b0784e7cf2

📥 Commits

Reviewing files that changed from the base of the PR and between ab35368 and 4227973.

📒 Files selected for processing (2)
  • shortcuts/sheets/data/flag-schemas.json
  • shortcuts/sheets/lark_sheet_write_cells_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • shortcuts/sheets/lark_sheet_write_cells_test.go

Comment on lines +465 to +479
"description": "@提及类型编号(仅 type='mention' 时可选)。0 或不填=@用户;@文件时按类型取:1=文档 3=电子表格 8=多维表格 11=思维笔记 12=文件 15=旧版幻灯片 16=知识库 22=新版文档 30=幻灯片 38=画板",
"type": "number",
"enum": [
0,
1,
3,
8,
11,
12,
15,
16,
22,
30,
38
]

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Schema enum is missing valid mention_type values required by the proto contract.

At Line 467-Line 478, the enum omits 2, 4, and 5, while the stated MENTION_FILE_TYPE allowlist includes them. Because schema validation is enforced before execution, valid mentions with those values will now be rejected at parse time.

Proposed fix
- "description": "`@提及类型编号`(仅 type='mention' 时可选)。0 或不填=`@用户`;@文件时按类型取:1=文档 3=电子表格 8=多维表格 11=思维笔记 12=文件 15=旧版幻灯片 16=知识库 22=新版文档 30=幻灯片 38=画板",
+ "description": "`@提及类型编号`(仅 type='mention' 时可选)。0 或不填=`@用户`;@文件时按类型编号取值(详见 enum 列表)。",
 "type": "number",
 "enum": [
 0,
 1,
+ 2,
 3,
+ 4,
+ 5,
 8,
 11,
 12,
 15,
 16,
 22,
 30,
 38
 ]
🤖 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/data/flag-schemas.json` around lines 465 - 479, The enum for
the mention type is missing values required by the proto (add 2, 4, and 5);
update the "enum" array tied to the property whose "description" begins with
"`@提及类型编号`(仅 type='mention' 时可选)" so it includes
0,1,2,3,4,5,8,11,12,15,16,22,30,38, ensuring the schema's allowed mention_type
values match the MENTION_FILE_TYPE allowlist and will not reject valid mentions
at parse time.

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

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /