fix: reduce base shortcut token overhead#1353
Closed
zhouyue-bytedance wants to merge 11 commits into
Closed
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds a new ChangesBase field-list-batch and field search updates
Drive import verification output
Documentation updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
github-actions
Bot
added
domain/base
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6fb3d8da998f7f8f4f9c3cfe8176f20b26f1309a🧩 Skill updatenpx skills add zhouyue-bytedance/cli#github-base-token-improve -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1353 +/- ##
==========================================
+ Coverage 71.58% 72.03% +0.44%
==========================================
Files 689 700 +11
Lines 65521 66395 +874
==========================================
+ Hits 46901 47825 +924
+ Misses 14972 14857 -115
- Partials 3648 3713 +65 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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
🧹 Nitpick comments (1)
shortcuts/base/base_execute_test.go (1)
858-887: ⚡ Quick winAssert the top-level multi-table
totalto lock output contract.The new multi-table test checks table payload content but doesn’t verify the aggregate
total, so regressions in that field can slip through unnoticed.Suggested test assertion
got := stdout.String() - if !strings.Contains(got, `"tables"`) || !strings.Contains(got, `"table_id": "tbl_a"`) || !strings.Contains(got, `"table_id": "tbl_b"`) || !strings.Contains(got, `"options": [`) || !strings.Contains(got, `"Todo"`) || strings.Contains(got, `"style"`) || strings.Contains(got, `"color"`) { + if !strings.Contains(got, `"tables"`) || !strings.Contains(got, `"table_id": "tbl_a"`) || !strings.Contains(got, `"table_id": "tbl_b"`) || !strings.Contains(got, `"total": 2`) || !strings.Contains(got, `"options": [`) || !strings.Contains(got, `"Todo"`) || strings.Contains(got, `"style"`) || strings.Contains(got, `"color"`) { t.Fatalf("stdout=%s", got) }🤖 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/base/base_execute_test.go` around lines 858 - 887, The test "list multiple tables" doesn't assert the aggregate/top-level total in the multi-table response; update the test (within the t.Run block that calls runShortcut with BaseFieldList and stdout) to verify the top-level "total" equals the expected combined count (e.g., 2) by either parsing stdout as JSON and asserting total == 2 or checking stdout contains the string `"total": 2`; reference the variables reg.Register, runShortcut, BaseFieldList and stdout to locate and modify the assertion.
🤖 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/base/field_ops.go`:
- Around line 262-268: The fieldSearchOptionsRef function returns a field
reference that may contain leading/trailing whitespace because only the
emptiness check trims it; update the function to trim the resolved value before
returning (i.e., call strings.TrimSpace on the final fieldRef obtained from
runtime.Str("field-id") or runtime.Str("field-name")) so the returned string is
normalized and safe to use when building the /fields/{ref}/options request path.
- Around line 164-173: The helper fieldListTableRefs forwards raw values from
runtime.StrArray("table-id") which can include duplicates and space-padded
entries; update fieldListTableRefs to normalize each ref by trimming whitespace,
ignore empty strings, and deduplicate while preserving order (use a seen map)
before returning; if the array is empty fall back to baseTableID() (also
normalize that value) so all returned refs are normalized and unique.
---
Nitpick comments:
In `@shortcuts/base/base_execute_test.go`:
- Around line 858-887: The test "list multiple tables" doesn't assert the
aggregate/top-level total in the multi-table response; update the test (within
the t.Run block that calls runShortcut with BaseFieldList and stdout) to verify
the top-level "total" equals the expected combined count (e.g., 2) by either
parsing stdout as JSON and asserting total == 2 or checking stdout contains the
string `"total": 2`; reference the variables reg.Register, runShortcut,
BaseFieldList and stdout to locate and modify the assertion.
🪄 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: 8f9e64d6-0892-4b8c-bbba-1026e01dcfb2
📒 Files selected for processing (23)
shortcuts/base/base_dryrun_ops_test.goshortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/field_list.goshortcuts/base/field_ops.goshortcuts/base/field_search_options.goshortcuts/base/record_delete.goshortcuts/base/record_get.goshortcuts/base/record_list.goshortcuts/base/record_ops.goshortcuts/base/record_query.goshortcuts/base/record_search.goshortcuts/base/record_share_link_create.goshortcuts/base/table_ops.goshortcuts/drive/drive_export_common.goshortcuts/drive/drive_export_test.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_test.goskills/lark-base/SKILL.mdskills/lark-base/references/dashboard-block-data-config.mdskills/lark-base/references/lark-base-dashboard-usecase.mdskills/lark-base/references/lark-base-dashboard.mdskills/lark-drive/references/lark-drive-import.md
Comment on lines
+164
to
+173
| func fieldListTableRefs(runtime *common.RuntimeContext) []string { | ||
| refs := runtime.StrArray("table-id") | ||
| if len(refs) == 0 { | ||
| ref := baseTableID(runtime) | ||
| if ref != "" { | ||
| refs = []string{ref} | ||
| } | ||
| } | ||
| return refs | ||
| } |
There was a problem hiding this comment.
Normalize and deduplicate --table-id refs before issuing requests.
Line 165 currently forwards raw array values, so repeated/space-padded refs can generate duplicate or malformed field-list calls and inflate token usage.
Suggested fix
func fieldListTableRefs(runtime *common.RuntimeContext) []string {
- refs := runtime.StrArray("table-id")
- if len(refs) == 0 {
- ref := baseTableID(runtime)
- if ref != "" {
- refs = []string{ref}
- }
- }
- return refs
+ raw := runtime.StrArray("table-id")
+ refs := make([]string, 0, len(raw))
+ seen := make(map[string]struct{}, len(raw))
+ for _, ref := range raw {
+ ref = strings.TrimSpace(ref)
+ if ref == "" {
+ continue
+ }
+ if _, ok := seen[ref]; ok {
+ continue
+ }
+ seen[ref] = struct{}{}
+ refs = append(refs, ref)
+ }
+ if len(refs) == 0 {
+ ref := strings.TrimSpace(baseTableID(runtime))
+ if ref != "" {
+ refs = []string{ref}
+ }
+ }
+ return refs
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func fieldListTableRefs(runtime *common.RuntimeContext) []string { | |
| refs := runtime.StrArray("table-id") | |
| if len(refs) == 0 { | |
| ref := baseTableID(runtime) | |
| if ref != "" { | |
| refs = []string{ref} | |
| } | |
| } | |
| return refs | |
| } | |
| func fieldListTableRefs(runtime *common.RuntimeContext) []string { | |
| raw := runtime.StrArray("table-id") | |
| refs := make([]string, 0, len(raw)) | |
| seen := make(map[string]struct{}, len(raw)) | |
| for _, ref := range raw { | |
| ref = strings.TrimSpace(ref) | |
| if ref == "" { | |
| continue | |
| } | |
| if _, ok := seen[ref]; ok { | |
| continue | |
| } | |
| seen[ref] = struct{}{} | |
| refs = append(refs, ref) | |
| } | |
| if len(refs) == 0 { | |
| ref := strings.TrimSpace(baseTableID(runtime)) | |
| if ref != "" { | |
| refs = []string{ref} | |
| } | |
| } | |
| return refs | |
| } |
🤖 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/base/field_ops.go` around lines 164 - 173, The helper
fieldListTableRefs forwards raw values from runtime.StrArray("table-id") which
can include duplicates and space-padded entries; update fieldListTableRefs to
normalize each ref by trimming whitespace, ignore empty strings, and deduplicate
while preserving order (use a seen map) before returning; if the array is empty
fall back to baseTableID() (also normalize that value) so all returned refs are
normalized and unique.
zgz2048
reviewed
Jun 9, 2026
| recordFilterFlag(), | ||
| {Name: recordFilterAliasFlag, Hidden: true, Input: []string{common.File}}, | ||
| recordSortFlag(), | ||
| {Name: recordSortAliasFlag, Hidden: true, Input: []string{common.File}}, |
Collaborator
Author
There was a problem hiding this comment.
模型会把 sort-json参数误写成sort，为了减少失败重试轮次，认为sort也作为合法参数
zgz2048
reviewed
Jun 9, 2026
| {Name: "record-id", Type: "string_array", Desc: "record ID (repeatable)"}, | ||
| {Name: "record-ids", Type: "string_array", Hidden: true}, | ||
| {Name: "field-id", Type: "string_array", Desc: "field ID or name to project; repeat to keep only needed columns"}, | ||
| {Name: "field-names", Type: "string_array", Hidden: true}, |
Collaborator
There was a problem hiding this comment.
为什么是string_array？这个 flag 直觉上是输入 --field-names '["field1", ...]' 而不是 --field-names field1
Collaborator
There was a problem hiding this comment.
此 mr 引入的所有 -ids -names 都有这个问题，其他的 flag 不重复评论
Collaborator
Author
There was a problem hiding this comment.
field-names等价于field-id，模型会把field-id误写为field-names
zgz2048
reviewed
Jun 9, 2026
|
|
||
| 硬规则： | ||
|
|
||
| - 删除并用 `+dashboard-get` 复核 `not_found` 后，用户只回复“确认/好的/收到”视为结束，不要再次创建或保留同名/正式版仪表盘。 |
Collaborator
There was a problem hiding this comment.
用户只回复“确认/好的/收到”视为结束 这个约束的作用是什么？看起来非常反常
Collaborator
Author
There was a problem hiding this comment.
你说的对，这个应该是针对评测集过拟合了。
zgz2048
reviewed
Jun 9, 2026
| 硬规则： | ||
|
|
||
| - 删除并用 `+dashboard-get` 复核 `not_found` 后，用户只回复“确认/好的/收到”视为结束，不要再次创建或保留同名/正式版仪表盘。 | ||
| - 用户只说“更新标题”但未给新标题时，可基于原名生成一次新标题；先用 `+dashboard-list` 避开已存在名称，遇同名冲突换名更新，不要创建新仪表盘。 |
zgz2048
reviewed
Jun 9, 2026
| if !runtime.Bool("full") { | ||
| fields = compactFields(fields) | ||
| } | ||
| results = append(results, map[string]interface{}{ |
Collaborator
There was a problem hiding this comment.
这一个 shortcut 在不同输入的情况下，返回的数据结构不一致，ai 容易误处理返回值
Collaborator
Author
There was a problem hiding this comment.
因为多个场景，都会需要base下所有表的schema，现在做法得每个表 field-list，轮次比较多。所以提供一个批量接口。在field-list接口加批量不合适的话，新加一个接口？
- revert field-list multi-table change; add dedicated +field-list-batch shortcut - make compact field output opt-in via --compact, default stays full (avoid breaking change) - compact output now keeps style - SKILL.md: drop 执行节奏/导出导入快路径 sections, restore Dashboard/Workflow/Role wording, sink new guidance into dashboard/workflow references - SKILL.md: add glossary (base-block vs dashboard-block disambiguation), batch-execution and help-first rules - field-json: note auto_number rule updates apply to existing records by default
zhouyue-bytedance
force-pushed
the
github-base-token-improve
branch
from
June 10, 2026 09:18
6fb3d8d to
3a8e86d
Compare
…rt 改造 Change-Id: Ie7fc9ff22509ec4b69b04f3c3b6c58cfbeb2ca88
zgz2048
reviewed
Jun 10, 2026
| 示例——一次取多个视图的配置： | ||
|
|
||
| ```bash | ||
| for v in vewAAA vewBBB vewCCC; do |
Collaborator
There was a problem hiding this comment.
发现用这个 script 反而 token 数比使用 field-list-batch 更省。并且调用风格更统一，ai 可以凭直觉重复
zgz2048
reviewed
Jun 10, 2026
|
|
||
| 默认值 / 约束： | ||
| - `style.rules` 是规则数组，数量 `1..9` | ||
| - `+field-update` 修改编号规则时，**默认会把新规则应用到已有记录** |
Collaborator
There was a problem hiding this comment.
建议加到 AutoNumber 字段的描述下，修改自动编号规则时，会自动更新所有已有单元格
Change-Id: Iaacc2ff5211e34f121aed46f01b0646ea7ed71b1
github-actions
Bot
added
size/XL
Change-Id: Ia577b7c10b945b06e741bb5463c23f4fe67838d0
- slim lark-base SKILL.md 6105->4719 tokens: drop 保留Reference chapter, condense caveat sections, sink write rules into cell-value.md; make help-first rule conditional for trivial commands - split formula-field-guide.md (9744->7243 tokens): move 37 rare functions to formula-functions-extended.md, move examples/requirement-translation to formula-examples.md; compress table padding - suggest: add containment signal so namespace-dropping guesses (+block-list -> +base-block-list) get did-you-mean hints - workflow docs: state that "add OR modify with same condition" is a single ChangeRecordTrigger workflow at guide/schema/leaf levels - batch-execution note: scripts should print counts/ids/failures only, not full payloads
…e-bytedance/cli into github-base-token-improve # Conflicts: # skills/lark-base/SKILL.md # skills/lark-base/references/lark-base-workflow-guide.md
…cuts Evidence-driven from 3 eval rounds of actual agent flag guesses: - projection aliases --field-names/--fields on record-list/search/get; --record-ids on record-get; --query on field-search-options; --filter/--sort on record-list/search (canonical+alias rejects with "use only one") - plural aliases accept repeated values, JSON arrays and comma-separated ids (comma split guarded by id prefix to protect names with commas) - cross-source dedupe: same value via canonical flag and alias is sent once; in-source duplicates still rejected downstream - misuse hints: record-list --json and data-query --table-id now return actionable errors; dashboard-block-get-data accepts --dashboard-id - SKILL.md: reorder 批量执行 before 善用 help, conditional help wording
Eval traces show comma-joined values on --field-names/--fields/--record-ids are exclusively lists (mostly field names), so the fld/rec prefix guard blocked every real usage and a trailing comma broke id lists too. Rule is now: singular flags stay literal (escape hatch for names containing a comma), plural aliases treat ASCII comma as separator; fullwidth "，" is never split.
Collaborator
Author
|
Superseded by a new PR from the same branch — recreating so pkg.pr.new regenerates the install command against the current head (the old pinned commit 6fb3d8d was orphaned by a force-push). |
6 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
3 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
Reduce token overhead for Lark Base and Drive shortcut flows by improving field/table/record request shaping and documenting lower-token dashboard/import usage patterns.
Changes
Test Plan
LARK_CLI_NO_PROXY= make unit-testlark-cli <domain> <command>flow works as expectedgo test ./shortcuts/base ./shortcuts/drivego vet ./...gofmt -l .go mod tidywith nogo.mod/go.sumdiffgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=refs/remotes/github-upstream/mainRelated Issues
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Documentation