fix: reduce base shortcut token overhead#1426
Open
zhouyue-bytedance wants to merge 20 commits into
Open
Conversation
- 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
…rt 改造 Change-Id: Ie7fc9ff22509ec4b69b04f3c3b6c58cfbeb2ca88
Change-Id: Iaacc2ff5211e34f121aed46f01b0646ea7ed71b1
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.
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds field batch listing, record alias handling, drive import verification output, shared Base token resolution, broader conditional scopes, and major Base/workflow/dashboard documentation updates. ChangesBase CLI rollout and reference refresh
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/suggest/suggest.go (1)
55-120: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd focused tests for the new containment ranking.
This changes the user-visible suggestion order for unknown subcommands via
cmd/root.go, but there’s no companion test in this cohort locking the new containment precedence and the 5-rune cutoff. A small table test around cases like+block-list→+base-block-list, plus short-fragment rejection, would keep future scoring tweaks from silently changing CLI hints.As per coding guidelines, "Every behavior change needs a test alongside the change".
🤖 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 `@internal/suggest/suggest.go` around lines 55 - 120, Add unit tests that lock the new containment-ranking and 5-rune cutoff: create a table-driven test that calls Closest with candidate sets demonstrating (1) a containment match like "+block-list" vs "+base-block-list" and asserts the contained candidate is ranked higher, (2) short-fragment rejection where a fragment under 5 runes (e.g., "list") does not trigger containsSegment matching, and (3) a few ties to ensure prefix/dist fallbacks behave; use containsSegment and editLimit semantics in assertions so future changes to Closest, containsSegment or editLimit will fail the tests if ordering or cutoff changes.Source: Coding guidelines
shortcuts/base/field_ops.go (1)
350-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t echo the merged ref into both
field_idandfield_name.After Lines 353-376 route
--field-idand--field-namethrough the samefieldRef, the response now mislabels one of those fields on every alias path. For example,--field-name Statusemits"field_id":"Status", and--field-id fld_xemits"field_name":"fld_x". That is an output-contract break for scripts consuming this JSON.Suggested direction
func executeFieldSearchOptions(runtime *common.RuntimeContext) error { baseToken := runtime.Str("base-token") tableIDValue := baseTableID(runtime) + fieldID := strings.TrimSpace(runtime.Str("field-id")) + fieldName := strings.TrimSpace(runtime.Str("field-name")) fieldRef := fieldSearchOptionsRef(runtime) params := map[string]interface{}{ "offset": runtime.Int("offset"), "limit": runtime.Int("limit"), } @@ - runtime.Out(map[string]interface{}{ - "field_id": fieldRef, - "field_name": fieldRef, - "keyword": fieldSearchOptionsKeyword(runtime), - "options": options, - "total": total, - }, nil) + out := map[string]interface{}{ + "keyword": fieldSearchOptionsKeyword(runtime), + "options": options, + "total": total, + } + if fieldID != "" { + out["field_id"] = fieldID + } + if fieldName != "" { + out["field_name"] = fieldName + } + runtime.Out(out, nil) return nil }🤖 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 350 - 379, The output currently writes the merged alias (fieldSearchOptionsRef) into both "field_id" and "field_name"; restore correct values by emitting the original input values instead of the merged ref: in executeFieldSearchOptions, capture the raw inputs (e.g. runtime.Str("field-id") and runtime.Str("field-name") or their existing accessor helpers) and set "field_id" to the raw field-id input and "field_name" to the raw field-name input (optionally falling back to fieldRef only when the specific raw input is empty), leaving baseV3 call and option processing unchanged.
🧹 Nitpick comments (4)
shortcuts/drive/drive_import_test.go (1)
794-803: ⚡ Quick winAdd an assertion for
verification_urlin the target-token output test.This test validates the new verification fields, but it skips
verification_url, which is also emitted by the updated execute path. Adding that check will catch URL-regression bugs in this exact flow.As per coding guidelines, “Every behavior change needs a test alongside the change.”
🤖 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/drive/drive_import_test.go` around lines 794 - 803, Add an assertion that data["verification_url"] exists and is a string and validate its value (e.g., that it contains the target token "bascn_target" and looks like a URL) in the same test where verification_token/target_token and verify_hint are checked; locate the checks around the data map in drive_import_test.go (the variables data, verification_token, target_token, verify_hint) and insert a check like: cast data["verification_url"] to string, ensure it's non-empty, contains "bascn_target" and starts with "http" to catch URL regressions.Source: Coding guidelines
skills/lark-base/references/formula-examples.md (1)
20-20: ⚡ Quick winAdd language specifiers to fenced code blocks for markdown linting compliance.
Three code blocks are missing language specifiers (MD040 violations):
- Line 20: Formula block
- Line 54: JSON block
- Line 83: Formula block
🔧 Proposed fix to add language specifiers
- Line 20, change: - ``` + ```formula - Line 54, change: - ```json + ```json - Line 83, change: - ``` + ```formulaAlso applies to: 54-54, 83-83
🤖 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 `@skills/lark-base/references/formula-examples.md` at line 20, Three fenced code blocks are missing language specifiers; update each triple-backtick fence so the first formula block becomes ```formula, the JSON block becomes ```json, and the second formula block becomes ```formula; locate the three code fences currently written as ``` (the formula examples and the JSON example) and change their opening fences to include the appropriate language tag and leave the rest of the block content unchanged to satisfy MD040 linting.Source: Linters/SAST tools
skills/lark-base/references/formula-field-guide.md (1)
357-363: ⚡ Quick winFix adverb repetition in FILTER+aggregation vs COUNTIF/SUMIF comparison table.
Line 362 (LanguageTool: ADVERB_REPETITION_PREMIUM) repeats "only" in the "When to use" row. Reword for clarity and conciseness.
✏️ Proposed fix to remove redundant adverb
Current (line 357–363):
| When to use | Conditions span multiple fields, or multi-step needed | Conditions only involve column values (e.g. `CurrentValue > 100`) |Suggestion: Remove "only" from the second branch since "only column values" is already clear from context. Alternatively, restructure to:
| When to use | Conditions span multiple fields, or multi-step logic needed | Conditions involve column values alone (e.g. `CurrentValue > 100`) |🤖 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 `@skills/lark-base/references/formula-field-guide.md` around lines 357 - 363, In the "FILTER+aggregation vs COUNTIF/SUMIF" comparison table, update the "When to use" row to remove the redundant adverb "only" in the COUNTIF/SUMIF column; e.g., change the COUNTIF/SUMIF cell from "Conditions only involve column values (e.g. `CurrentValue > 100`)" to a concise alternative such as "Conditions involve column values (e.g. `CurrentValue > 100`)" or "Conditions involve column values alone (e.g. `CurrentValue > 100`)"; edit the table row labeled "When to use" to apply this wording change.Source: Linters/SAST tools
skills/lark-base/references/workflow-steps/system-loop.md (1)
19-21: 💤 Low valueRemove extra horizontal separator line.
Lines 19-21 contain a double separator (
---appears twice). Consolidate to a single separator before the "相关" section.✏️ Proposed fix
| `max_loop_times` | 否 | 最大循环次数 | --- ---- ## 相关🤖 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 `@skills/lark-base/references/workflow-steps/system-loop.md` around lines 19 - 21, Remove the duplicate horizontal rule: in the markdown file where two consecutive '---' separators appear before the "相关" section, delete one so only a single '---' remains (locate the double '---' block and reduce it to a single separator).
🤖 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/base_shortcuts_test.go`:
- Around line 72-77: Update the negative-path assertions in
TestFieldSearchOptionsRequiresFieldRef (and the similar case in
shortcuts/base/helpers_test.go) to assert typed error metadata instead of only
substring matching: call errs.ProblemOf(err) and assert the returned Problem has
the expected category/subtype/param, and use errors.As(err, &ve) where ve is
*errs.ValidationError to assert ve.Param equals the expected param; keep the
Validate call via BaseFieldSearchOptions.Validate(...) but replace the
strings.Contains check with these typed checks using errs.ProblemOf and
errors.As for precise validation metadata assertions.
In `@shortcuts/base/field_list.go`:
- Line 24: Add a direct test that exercises the single-table command path for
the compact flag: create a unit/integration test that calls executeFieldList
(the handler that backs `+field-list`) with the `--compact` flag and verifies
the returned field objects are compact (contain only id/name/type/style/options)
and that the default/full payload behavior remains unchanged; ensure the new
test lives alongside existing tests (not just the batch tests) and asserts both
compact output structure and no regression in non-compact output for
executeFieldList.
In `@shortcuts/base/field_ops.go`:
- Around line 203-246: The current isBaseTableID (used by
resolveFieldListTableRefs) treats any string starting with "tbl" as an ID,
letting valid names like "tbl_orders" bypass name resolution; change
isBaseTableID to strictly validate the expected ID pattern (e.g., "tbl" followed
only by alphanumeric chars/digits) instead of just HasPrefix: implement a regex
or check that ref length > 3, strings.HasPrefix(ref, "tbl") and every rune in
ref[3:] is alphanumeric, and return true only in that case so
resolveFieldListTableRefs will call resolveTableRef for names like "tbl_orders".
In `@shortcuts/base/record_ops.go`:
- Around line 193-213: normalizePluralReferenceValues currently treats a value
that starts with "[" as CSV when json.Unmarshal fails, producing garbage refs;
change normalizePluralReferenceValues to return ([]string, error) and on
json.Unmarshal error return a descriptive error (including the offending value
and the parse error) instead of falling back to comma-splitting, then update all
callers to handle/propagate that error so malformed JSON-array aliases are
surfaced and not sent to the API.
In `@skills/lark-base/references/lark-base-dashboard.md`:
- Around line 24-30: The walkthrough still shows repeated +field-list calls;
update those snippets to use a single +field-list-batch invocation that accepts
multiple --table-id arguments (e.g., +field-list-batch --table-id <表1>
--table-id <表2>), replace any loops or multiple examples that call +field-list
in the walkthrough with the consolidated batch call, adjust example output and
any notes about token usage accordingly, and ensure any references to downstream
steps (e.g., subsequent +dashboard-arrange or +dashboard-block-create examples)
remain consistent with the batched field result.
In `@skills/lark-base/references/workflow-steps/common-types-and-refs.md`:
- Line 28: Update the two untyped fenced code blocks in common-types-and-refs.md
that contain the example reference paths like "$.{...}" so their opening fences
include a language identifier (e.g., change ``` to ```text) to satisfy
markdownlint and ensure consistent highlighting for the $.{...} examples.
In `@skills/lark-base/references/workflow-steps/trigger-change-record.md`:
- Around line 5-17: The example JSON uses the wrong field name "condition" —
update the example to use "condition_list" so it matches the documented field
and the workflow schema; ensure the example shows the same allowed types (null
or an array of AndCondition entries) and that any references to condition in
this file (e.g., in the example block next to `trigger_control_list`) are
renamed to `condition_list` to match SetRecordTrigger and schema contracts.
In `@skills/lark-base/references/workflow-steps/trigger-lark-message.md`:
- Line 20: Update the case of the receive_scene value in the workflow schema:
locate the definition that currently sets receive_scene = "Chat" (in the common
types/refs area) and change the string to lowercase "chat" so it matches the
case-sensitive schema and the example in trigger-lark-message.md; ensure any
related comparisons or validations reference receive_scene using lowercase
"group"/"chat".
In `@skills/lark-base/SKILL.md`:
- Around line 48-50: The glossary rows for "Dashboard（仪表盘）" and "Chart（图表/组件）"
have their ID prefixes reversed: update the "Dashboard（仪表盘）" row to indicate IDs
start with `cht` (or the dashboard prefix used in commands like `dash_1`) and
update the "Chart（图表/组件）" / "Dashboard block" row to indicate IDs start with
`blk` (matching block IDs like `blk_1` used in tests/commands); keep the Base
block (`+base-block-*`) row unchanged. Ensure you edit the lines containing the
headers "Dashboard（仪表盘）" and "Chart（图表/组件）" so the prefixes `blk` and `cht` are
swapped to match the PR usage.
---
Outside diff comments:
In `@internal/suggest/suggest.go`:
- Around line 55-120: Add unit tests that lock the new containment-ranking and
5-rune cutoff: create a table-driven test that calls Closest with candidate sets
demonstrating (1) a containment match like "+block-list" vs "+base-block-list"
and asserts the contained candidate is ranked higher, (2) short-fragment
rejection where a fragment under 5 runes (e.g., "list") does not trigger
containsSegment matching, and (3) a few ties to ensure prefix/dist fallbacks
behave; use containsSegment and editLimit semantics in assertions so future
changes to Closest, containsSegment or editLimit will fail the tests if ordering
or cutoff changes.
In `@shortcuts/base/field_ops.go`:
- Around line 350-379: The output currently writes the merged alias
(fieldSearchOptionsRef) into both "field_id" and "field_name"; restore correct
values by emitting the original input values instead of the merged ref: in
executeFieldSearchOptions, capture the raw inputs (e.g. runtime.Str("field-id")
and runtime.Str("field-name") or their existing accessor helpers) and set
"field_id" to the raw field-id input and "field_name" to the raw field-name
input (optionally falling back to fieldRef only when the specific raw input is
empty), leaving baseV3 call and option processing unchanged.
---
Nitpick comments:
In `@shortcuts/drive/drive_import_test.go`:
- Around line 794-803: Add an assertion that data["verification_url"] exists and
is a string and validate its value (e.g., that it contains the target token
"bascn_target" and looks like a URL) in the same test where
verification_token/target_token and verify_hint are checked; locate the checks
around the data map in drive_import_test.go (the variables data,
verification_token, target_token, verify_hint) and insert a check like: cast
data["verification_url"] to string, ensure it's non-empty, contains
"bascn_target" and starts with "http" to catch URL regressions.
In `@skills/lark-base/references/formula-examples.md`:
- Line 20: Three fenced code blocks are missing language specifiers; update each
triple-backtick fence so the first formula block becomes ```formula, the JSON
block becomes ```json, and the second formula block becomes ```formula; locate
the three code fences currently written as ``` (the formula examples and the
JSON example) and change their opening fences to include the appropriate
language tag and leave the rest of the block content unchanged to satisfy MD040
linting.
In `@skills/lark-base/references/formula-field-guide.md`:
- Around line 357-363: In the "FILTER+aggregation vs COUNTIF/SUMIF" comparison
table, update the "When to use" row to remove the redundant adverb "only" in the
COUNTIF/SUMIF column; e.g., change the COUNTIF/SUMIF cell from "Conditions only
involve column values (e.g. `CurrentValue > 100`)" to a concise alternative such
as "Conditions involve column values (e.g. `CurrentValue > 100`)" or "Conditions
involve column values alone (e.g. `CurrentValue > 100`)"; edit the table row
labeled "When to use" to apply this wording change.
In `@skills/lark-base/references/workflow-steps/system-loop.md`:
- Around line 19-21: Remove the duplicate horizontal rule: in the markdown file
where two consecutive '---' separators appear before the "相关" section, delete
one so only a single '---' remains (locate the double '---' block and reduce it
to a single separator).
🪄 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: 620a9a1c-2e92-41ea-a1b5-8aac4f0f4270
📒 Files selected for processing (50)
internal/suggest/suggest.goshortcuts/base/base_data_query.goshortcuts/base/base_dryrun_ops_test.goshortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/dashboard_block_get_data.goshortcuts/base/field_list.goshortcuts/base/field_list_batch.goshortcuts/base/field_ops.goshortcuts/base/field_search_options.goshortcuts/base/helpers_test.goshortcuts/base/record_get.goshortcuts/base/record_list.goshortcuts/base/record_ops.goshortcuts/base/record_query.goshortcuts/base/record_search.goshortcuts/base/shortcuts.goshortcuts/base/table_ops.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/formula-examples.mdskills/lark-base/references/formula-field-guide.mdskills/lark-base/references/formula-functions-extended.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-dashboard-usecase.mdskills/lark-base/references/lark-base-dashboard.mdskills/lark-base/references/lark-base-field-json.mdskills/lark-base/references/lark-base-workflow-guide.mdskills/lark-base/references/lark-base-workflow-schema.mdskills/lark-base/references/workflow-steps/action-add-record.mdskills/lark-base/references/workflow-steps/action-delay.mdskills/lark-base/references/workflow-steps/action-find-record.mdskills/lark-base/references/workflow-steps/action-generate-ai-text.mdskills/lark-base/references/workflow-steps/action-http-client.mdskills/lark-base/references/workflow-steps/action-lark-message.mdskills/lark-base/references/workflow-steps/action-set-record.mdskills/lark-base/references/workflow-steps/branch-if-else.mdskills/lark-base/references/workflow-steps/branch-switch.mdskills/lark-base/references/workflow-steps/common-types-and-refs.mdskills/lark-base/references/workflow-steps/system-loop.mdskills/lark-base/references/workflow-steps/trigger-add-record.mdskills/lark-base/references/workflow-steps/trigger-button.mdskills/lark-base/references/workflow-steps/trigger-change-record.mdskills/lark-base/references/workflow-steps/trigger-lark-message.mdskills/lark-base/references/workflow-steps/trigger-reminder.mdskills/lark-base/references/workflow-steps/trigger-set-record.mdskills/lark-base/references/workflow-steps/trigger-timer.mdskills/lark-drive/references/lark-drive-import.md
…token-improve # Conflicts: # skills/lark-base/SKILL.md
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1426 +/- ##
==========================================
+ Coverage 72.83% 73.43% +0.60%
==========================================
Files 732 751 +19
Lines 69140 69568 +428
==========================================
+ Hits 50356 51088 +732
+ Misses 15003 14711 -292
+ Partials 3781 3769 -12 ☔ 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@1cca939881b8ffaf8571877076ab534705c4de3b🧩 Skill updatenpx skills add zhouyue-bytedance/cli#github-base-token-improve -y -g |
… skill docs record pagination: - record-list / record-search accept a hidden --page-size alias for --limit and a hidden --page-token flag that hints back to --offset/--limit, so the im-domain pagination habits agents carry over no longer error out - add recordPageLimit / validateRecordPageLimit and route list/search/dry-run through them; reject passing both --limit and --page-size - cover the alias, page-size-only and page-token paths in helpers_test dashboard skill docs: - rename dashboard-block-data-config.md to lark-base-dashboard-block-data-config.md and update all flag descs, tips, validation errors and tests to the new name - slim lark-base-dashboard.md and split the usecase guide into lark-base-dashboard-write.md; SKILL.md routes dashboard read/write/chart tasks to the right file SKILL.md hints: - mark record-get/list/search as markdown-by-default, add --format json only when piping/parsing - steer multi-field reads to +field-list, reserve +field-get for a single field
…verhead and validation fixes base-token resolution: - --base-token now accepts a raw base token, a /base/ URL, or a /wiki/ URL; wiki URLs resolve to the underlying bitable via wiki get_node, gated by a conditional wiki:node:retrieve scope on the base commands - resolution is memoized per command, so a wiki URL triggers at most one get_node call however many times the token is referenced - baseV3 calls surface a resolution failure (e.g. a wiki URL that is not a bitable) with a clear error before issuing the request - base-get tip updated to describe the accepted token/URL forms output token overhead: - +field-list defaults to full field objects again; compact projection is now opt-in via --compact, and the multi-table total sums per-table field counts - +workflow-list gains --compact for lower-context workflow_id/title/status output validation: - record query rejects passing a canonical flag and its deprecated alias together (--filter-json / --sort-json), matching the --limit/--page-size behavior
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/base/workflow_execute_test.go (1)
66-103: ⚡ Quick winAdd execute-path tests for URL-form
--base-tokenin workflow shortcuts.Line 66 adds compact-output coverage, but this layer also changed workflow commands to resolve
--base-tokenviabaseTokenOrRaw(runtime). Please add tests that exercise/base/and/wiki/inputs (including wiki resolution) so the new token-resolution behavior is covered alongside this change.
As per coding guidelines, “Every behavior change needs a test alongside the change.”🤖 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/workflow_execute_test.go` around lines 66 - 103, The TestBaseWorkflowExecuteListCompact test covers compact output but does not test the new token-resolution behavior introduced by baseTokenOrRaw(runtime) in the workflow shortcuts. Add separate tests that exercise both /base/ and /wiki/ input paths with the --base-token parameter, including tests for wiki resolution behavior, to ensure the new token-resolution logic is properly covered alongside the behavioral changes made to the workflow command execution path.Source: Coding guidelines
🤖 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/view_ops.go`:
- Line 158: Replace all instances of baseTokenOrRaw(runtime) with
baseToken(runtime) in the execute paths and add immediate error returns. This
change applies to the listAllViews call at line 158 in executeViewList, and the
same pattern must be applied at lines 170, 183, 202, 214, 227, 244, 262, and 278
corresponding to executeViewGet, executeViewCreate, executeViewDelete,
executeViewGetProperty, executeViewSetJSONObject, executeViewSetWrapped,
executeViewSetVisibleFields, and executeViewRename. At each location, capture
the error returned by baseToken(runtime) and return it immediately rather than
falling back to the raw input, ensuring typed error classification is preserved
throughout the execution path.
---
Nitpick comments:
In `@shortcuts/base/workflow_execute_test.go`:
- Around line 66-103: The TestBaseWorkflowExecuteListCompact test covers compact
output but does not test the new token-resolution behavior introduced by
baseTokenOrRaw(runtime) in the workflow shortcuts. Add separate tests that
exercise both /base/ and /wiki/ input paths with the --base-token parameter,
including tests for wiki resolution behavior, to ensure the new token-resolution
logic is properly covered alongside the behavioral changes made to the workflow
command execution path.
🪄 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: 750a9449-00b9-4a07-b147-70f8cb3180a9
📒 Files selected for processing (92)
shortcuts/base/base_advperm_disable.goshortcuts/base/base_advperm_enable.goshortcuts/base/base_block_create.goshortcuts/base/base_block_delete.goshortcuts/base/base_block_list.goshortcuts/base/base_block_move.goshortcuts/base/base_block_ops.goshortcuts/base/base_block_rename.goshortcuts/base/base_command_common.goshortcuts/base/base_copy.goshortcuts/base/base_data_query.goshortcuts/base/base_form_create.goshortcuts/base/base_form_delete.goshortcuts/base/base_form_get.goshortcuts/base/base_form_list.goshortcuts/base/base_form_questions_create.goshortcuts/base/base_form_questions_delete.goshortcuts/base/base_form_questions_list.goshortcuts/base/base_form_questions_update.goshortcuts/base/base_form_submit.goshortcuts/base/base_form_update.goshortcuts/base/base_get.goshortcuts/base/base_ops.goshortcuts/base/base_role_create.goshortcuts/base/base_role_delete.goshortcuts/base/base_role_get.goshortcuts/base/base_role_list.goshortcuts/base/base_role_update.goshortcuts/base/base_shortcut_helpers.goshortcuts/base/dashboard_arrange.goshortcuts/base/dashboard_block_create.goshortcuts/base/dashboard_block_delete.goshortcuts/base/dashboard_block_get.goshortcuts/base/dashboard_block_get_data.goshortcuts/base/dashboard_block_list.goshortcuts/base/dashboard_block_update.goshortcuts/base/dashboard_create.goshortcuts/base/dashboard_delete.goshortcuts/base/dashboard_get.goshortcuts/base/dashboard_list.goshortcuts/base/dashboard_ops.goshortcuts/base/dashboard_update.goshortcuts/base/field_create.goshortcuts/base/field_delete.goshortcuts/base/field_get.goshortcuts/base/field_list.goshortcuts/base/field_ops.goshortcuts/base/field_search_options.goshortcuts/base/field_update.goshortcuts/base/helpers.goshortcuts/base/record_batch_create.goshortcuts/base/record_batch_update.goshortcuts/base/record_delete.goshortcuts/base/record_get.goshortcuts/base/record_history_list.goshortcuts/base/record_list.goshortcuts/base/record_ops.goshortcuts/base/record_search.goshortcuts/base/record_share_link_create.goshortcuts/base/record_upload_attachment.goshortcuts/base/record_upsert.goshortcuts/base/table_create.goshortcuts/base/table_delete.goshortcuts/base/table_get.goshortcuts/base/table_list.goshortcuts/base/table_ops.goshortcuts/base/table_update.goshortcuts/base/view_create.goshortcuts/base/view_delete.goshortcuts/base/view_get.goshortcuts/base/view_get_card.goshortcuts/base/view_get_filter.goshortcuts/base/view_get_group.goshortcuts/base/view_get_sort.goshortcuts/base/view_get_timebar.goshortcuts/base/view_get_visible_fields.goshortcuts/base/view_list.goshortcuts/base/view_ops.goshortcuts/base/view_rename.goshortcuts/base/view_set_card.goshortcuts/base/view_set_filter.goshortcuts/base/view_set_group.goshortcuts/base/view_set_sort.goshortcuts/base/view_set_timebar.goshortcuts/base/view_set_visible_fields.goshortcuts/base/workflow_create.goshortcuts/base/workflow_disable.goshortcuts/base/workflow_enable.goshortcuts/base/workflow_execute_test.goshortcuts/base/workflow_get.goshortcuts/base/workflow_list.goshortcuts/base/workflow_update.go
✅ Files skipped from review due to trivial changes (6)
- shortcuts/base/view_rename.go
- shortcuts/base/view_get_group.go
- shortcuts/base/record_delete.go
- shortcuts/base/view_get_sort.go
- shortcuts/base/view_set_filter.go
- shortcuts/base/base_command_common.go
🚧 Files skipped from review as they are similar to previous changes (8)
- shortcuts/base/dashboard_block_update.go
- shortcuts/base/dashboard_block_get_data.go
- shortcuts/base/record_get.go
- shortcuts/base/field_list.go
- shortcuts/base/field_search_options.go
- shortcuts/base/record_search.go
- shortcuts/base/record_list.go
- shortcuts/base/field_ops.go
yballul-bytedance
added a commit
to zhouyue-bytedance/cli
that referenced
this pull request
Jun 19, 2026
Drop overfitted comma-splitting and prefix heuristics, fix workflow trigger schema mismatches, restore deleted high-value workflow content, and clean up dead doc anchors flagged in the PR review. - Remove `normalizePluralReferenceValues` and the hidden `--record-ids` / `--field-names` / `--fields` aliases on `+record-get/list/search`; CSV-style values that contain user data (e.g. "收入,支出对比") are no longer silently split. Negative-path tests assert the aliases are rejected with `unknown flag`. - Remove `isBaseTableID`. `+field-list` / `+field-list-batch` now always resolve refs through `resolveTableRef`, so user table names like `tbl_orders` are no longer mistaken for table IDs. - `executeFieldSearchOptions` now emits only the key the user actually supplied (`field_id` or `field_name`) instead of stuffing the same value into both. Two new tests cover the by-id and by-name paths. - Workflow trigger docs: `trigger-change-record.md` example uses `condition_list` (was `condition`); all four record/reminder triggers call out that `condition_list: []` is rejected by the API and require `null` or omission. `common-types-and-refs.md` uses lowercase `receive_scene = "chat"`. - Workflow guide common-error table now headers itself as a frequency-ordered Top list so agents treat it as the first stop, and picks up four high-frequency rows that were dropped during the PR's doc split (`condition_list: []`, missing `client_token`, `Undefined Step Type`, select-with-text). - Restore the end-to-end IfElseBranch example (trigger → branch → branch actions → join → AI text) into `branch-if-else.md` so agents have a concrete model for `children.links` + `next` wiring. - Test fixtures use the real ID prefixes: dashboard IDs are `blk_*`, chart `block_id`s are `cht_*`. Fixes the dsh_/dash_/blk_a confusion flagged in the review. - Doc cleanup: re-link the orphan 717-line `lark-base-dashboard-block-get-data.md` from `lark-base-dashboard.md`; fix the dead "执行要点" anchor in `SKILL.md` (now points to `lark-base-dashboard-write.md`); add language tags to fenced code blocks (markdownlint MD040) and drop the duplicate `---` rule in `system-loop.md`. Change-Id: I9217be2ee0bc16f53c9462c8a9bd2d69b26f9ea1 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
Collaborator
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/suggest/suggest.go (1)
55-87:⚠️ Potential issue | 🟠 MajorAdd tests for containment matching and ranking in
Closest.The new
containsSegmentfunction and its integration into acceptance/ranking logic lack test coverage. No test exercises:
- Containment-based acceptance (the
ctcondition at line 72)- Containment ranking precedence over prefix/distance (the sort order at line 77–78)
- The code comment's example scenario (
"+block-list"vs"+base-block-list")Per coding guidelines, every behavior change requires a test. Add at least one test case verifying that candidates containing the input as a segment are accepted and ranked first.
🤖 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 `@internal/suggest/suggest.go` around lines 55 - 87, The Closest function now includes containsSegment-based acceptance and ranking logic, but there are no tests verifying this behavior. Add test cases for the Closest function that verify the containment-based acceptance condition (the ct variable), the ranking precedence where containment takes priority over prefix and distance in the sort logic, and specifically the example scenario mentioned in the code comment where "+block-list" should be accepted and ranked higher when searching for similar candidates like "+base-block-list". Ensure the tests cover all three aspects: acceptance via containsSegment, ranking precedence in the sort order, and the specific namespace prefix example.Source: Coding guidelines
shortcuts/base/workflow_get.go (1)
33-58:⚠️ Potential issue | 🟠 MajorAdd tests covering URL token resolution in workflow commands.
Tests for
+workflow-get,+workflow-enable, and+workflow-disablecurrently exercise only raw token inputs (e.g.,--base-token app_x). The code usesbaseTokenOrRaw()which includes logic to parse and resolve/base/URLs and/wiki/URLs via API calls. Tests should cover these URL resolution paths and the error fallback behavior to catch regressions in token handling, per the requirement that every behavior change needs a test.🤖 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/workflow_get.go` around lines 33 - 58, The test coverage for workflow commands (workflow-get, workflow-enable, and workflow-disable) currently only tests raw token inputs but does not cover the URL token resolution logic in baseTokenOrRaw() which handles parsing and resolving /base/ URLs and /wiki/ URLs via API calls. Add test cases for each of these three workflow commands that exercise the baseTokenOrRaw() function with URL inputs (both /base/ and /wiki/ URL patterns), verify successful token resolution from these URLs, and test the error fallback behavior when URL resolution fails, ensuring all code paths in token handling are covered by tests.Source: Coding guidelines
shortcuts/base/base_dashboard_execute_test.go (1)
388-394:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAssert typed error metadata in these error-path tests.
These touched error-path tests still assert nil/error-string behavior only. Please assert typed metadata (
category/subtype/param) and cause preservation for the returned typed error.Suggested pattern
+import ( + "errors" + ... + "github.com/larksuite/cli/errs" +) + +func assertInvalidArg(t *testing.T, err error, param string) { + t.Helper() + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got: %v", err) + } + if p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("unexpected problem: category=%v subtype=%v", p.Category, p.Subtype) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) || ve.Param != param { + t.Fatalf("expected validation param %q, got err=%v", param, err) + } + if errors.Unwrap(err) == nil { + t.Fatalf("expected wrapped cause to be preserved") + } +}As per coding guidelines,
**/*_test.go: “Error-path tests must assert typed metadata via errs.ProblemOf (category / subtype / param) and cause preservation, not message substrings alone.” Based on learnings,errs.ProblemOfdoes not exposeParam, soerrors.As(err, *errs.ValidationError)is needed to assertParam.Also applies to: 455-460, 628-676, 713-725
🤖 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_dashboard_execute_test.go` around lines 388 - 394, The error-path test for "invalid data-config json" currently only checks if an error is returned (err == nil), but does not assert typed error metadata. After capturing the error from the runShortcut call in the BaseDashboardBlockCreate test, add assertions using errs.ProblemOf to verify the error category and subtype, and use errors.As with errs.ValidationError to assert the Param field. Additionally verify that the error cause is properly preserved by checking the underlying error chain.Sources: Coding guidelines, Learnings
♻️ Duplicate comments (1)
shortcuts/base/view_ops.go (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate base-token resolution errors in execute paths instead of falling back to raw input.
baseTokenOrRaw(runtime)suppressesbaseToken(runtime)failures and continues with the unresolved raw flag value, which loses typed error classification and can produce downstream API-path failures. UsebaseToken(runtime)in execute paths and return the error immediately.As per coding guidelines: "When a lower layer already returns a typed error, pass it through unchanged to avoid downgrades in error classification."
Also applies to: 158-158, 170-170, 183-183, 202-202, 214-214, 227-227, 244-244, 262-262, 278-278
🤖 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/view_ops.go` at line 16, Replace all usages of baseTokenOrRaw(runtime) with baseToken(runtime) throughout the execute paths in view_ops.go (including the Set("base_token", baseTokenOrRaw(runtime)) call at line 16 and the other occurrences at lines 158, 170, 183, 202, 214, 227, 244, 262, and 278). Since baseToken(runtime) returns both a value and an error, capture the error return value and return it immediately from the current function instead of suppressing it with the fallback behavior, ensuring typed errors are properly propagated rather than downgraded to raw input values.Source: Coding guidelines
🧹 Nitpick comments (4)
skills/lark-base/SKILL.md (2)
43-43: 💤 Low valueAdjust Chinese measure word for consistency.
Line 43 uses "一张数据表" where "张" is not the conventional measure word for tables in this context. Consider changing to "一个数据表" or omitting the measure word for more natural phrasing.
🤖 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 `@skills/lark-base/SKILL.md` at line 43, In the Table row description where it currently reads "一张数据表", change the measure word "张" to "个" to make it "一个数据表" for consistency with conventional Chinese phrasing for data tables, or alternatively omit the measure word entirely to read simply "Base 内的数据表" for more natural wording. The current measure word "张" is typically used for flat objects like sheets or pictures, not for abstract data structures.Source: Linters/SAST tools
124-124: 💤 Low valueFix grammar in the permission-grant explanation.
Line 124 reads "并把用户是否可打开新 Base 告知用户," which has a grammatical issue with the "把" construction. Rewrite to something like "并向用户告知是否可打开新 Base" or restructure to avoid the awkward double "用户" reference.
🤖 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 `@skills/lark-base/SKILL.md` at line 124, The sentence in line 124 contains a grammatical issue with awkward repetition of "用户" (user) in the "把" construction. Restructure the text to improve clarity by removing the redundant "用户" reference. Replace the phrase "并把用户是否可打开新 Base 告知用户" with a more natural construction that conveys the same meaning without the double reference to the user, such as using "向用户告知是否可打开新 Base" or a similar alternative that maintains the original intent while improving grammar.Source: Linters/SAST tools
shortcuts/drive/drive_import_test.go (1)
775-786: ⚡ Quick winAvoid global working-directory mutation in this test.
Lines 775-779 call
os.Chdir, which changes global process state and can create cross-test interference. Use an absolute temp file path instead.Proposed refactor
- tmpDir := t.TempDir() - origDir, _ := os.Getwd() - if err := os.Chdir(tmpDir); err != nil { - t.Fatalf("Chdir: %v", err) - } - defer os.Chdir(origDir) - if err := os.WriteFile("snapshot.base", []byte("fake-base"), 0o644); err != nil { + tmpDir := t.TempDir() + snapshotPath := filepath.Join(tmpDir, "snapshot.base") + if err := os.WriteFile(snapshotPath, []byte("fake-base"), 0o644); err != nil { t.Fatalf("WriteFile: %v", err) } if err := mountAndRunDrive(t, DriveImport, []string{ - "+import", "--file", "snapshot.base", "--type", "bitable", "--target-token", "bascn_target", "--as", "user", + "+import", "--file", snapshotPath, "--type", "bitable", "--target-token", "bascn_target", "--as", "user", }, f, stdout); err != nil { t.Fatalf("import should succeed, got: %v", err) }🤖 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/drive/drive_import_test.go` around lines 775 - 786, The test is using os.Chdir to change the global working directory, which can cause interference between tests. Remove the origDir, _ := os.Getwd() call and the defer os.Chdir(origDir) statement. Instead, use an absolute path by joining tmpDir with "snapshot.base" in both the os.WriteFile call and when passing the filename to the mountAndRunDrive function call, so that the relative path "snapshot.base" is replaced with the full absolute path throughout this test block.shortcuts/base/helpers_test.go (1)
501-538: ⚡ Quick winConsider using typed error assertions instead of substring matching.
The validation tests use
strings.Contains(err.Error(), "...")to check error messages. Per coding guidelines for test files, error-path tests should assert typed metadata viaerrs.ProblemOf(category/subtype/param) rather than message substrings alone. This makes tests more robust against message changes and verifies the error classification.Example typed assertion pattern
func TestFieldSearchOptionsKeywordQueryAlias(t *testing.T) { ctx := context.Background() - if err := BaseFieldSearchOptions.Validate(ctx, newBaseTestRuntime( + err := BaseFieldSearchOptions.Validate(ctx, newBaseTestRuntime( map[string]string{"field-id": "Status", "keyword": "A", "query": "B"}, nil, nil, - )); err == nil || !strings.Contains(err.Error(), "use only one") { - t.Fatalf("err=%v", err) + )) + if err == nil { + t.Fatal("expected validation error for both keyword and query") + } + problem, ok := errs.ProblemOf(err) + if !ok || problem.Category != errs.CategoryValidation { + t.Fatalf("expected typed validation error, got: %v", err) }Apply similar pattern to
TestRecordPageSizeAlias.🤖 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/helpers_test.go` around lines 501 - 538, Replace the error message substring matching using strings.Contains(err.Error(), ...) in both TestFieldSearchOptionsKeywordQueryAlias and TestRecordPageSizeAlias test functions with typed error assertions using errs.ProblemOf to check error metadata (category/subtype/param) instead. This makes the tests more robust by verifying the actual error classification rather than relying on message text that can change, allowing you to assert the specific problem type returned by the validation methods like BaseFieldSearchOptions.Validate and BaseRecordList.Validate.Source: Coding guidelines
🤖 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/base_data_query.go`:
- Around line 34-36: Add a test case to verify that the BaseDataQuery.Validate
method properly rejects a non-empty table-id flag with typed error metadata.
Create a test that calls BaseDataQuery.Validate with a context and a runtime
containing both table-id set to a non-empty value and dsl set to a valid JSON
object, then use errs.ProblemOf to extract the error and assert that the
returned error has Category equal to errs.CategoryValidation and Subtype equal
to errs.SubtypeInvalidArgument. This ensures the validation at the
baseFlagErrorf call site for the table-id rejection is properly covered by a
test that checks typed error metadata rather than just the error message.
In `@shortcuts/base/base_execute_test.go`:
- Around line 1673-1688: Replace the string-based error assertions in the three
test cases (for BaseRecordList, BaseRecordSearch, and BaseRecordGet) that
currently use strings.Contains() to check for "unknown flag:" messages. Instead,
use typed error assertions with errs.ProblemOf to validate the error metadata
including the category, subtype, and param fields. This applies to the error
checks after the runShortcut calls for listErr, searchErr, and getErr
respectively, ensuring each assertion validates the structured error properties
rather than substring matching.
In `@shortcuts/base/base_role_delete.go`:
- Around line 52-53: The Execute method in base_role_delete.go currently uses
baseTokenOrRaw(runtime) which suppresses typed resolver errors by falling back
to raw tokens, causing error classification downgrades. Replace
baseTokenOrRaw(runtime) with baseToken(runtime) to allow typed errors to
propagate unchanged. Apply this same fix to the Execute methods in BaseRoleGet,
BaseRoleList, and BaseRoleUpdate where the same pattern appears.
In `@shortcuts/base/field_list_batch.go`:
- Around line 12-18: The BaseFieldListBatch shortcut definition is missing the
ConditionalScopes field that other similar read shortcuts include, creating
inconsistent authorization behavior. Add the missing ConditionalScopes field
with the value []string{"wiki:node:retrieve"} to the BaseFieldListBatch struct
initialization to align it with sibling read commands in the same rollout.
In `@skills/lark-base/references/lark-base-workflow-guide.md`:
- Around line 7-15: In the Markdown table row containing the workflow
enable/disable operation, locate the command option `<enabled|disabled>` within
backticks on the line with `+workflow-list --status`. The unescaped pipe
character is being interpreted as a table column delimiter and breaking the
table structure. Escape the pipe character by changing `<enabled|disabled>` to
`<enabled\|disabled>` so the Markdown parser treats it as literal text content
rather than a column separator.
In `@skills/lark-base/references/workflow-steps/common-types-and-refs.md`:
- Around line 1-5: The Markdown heading hierarchy is incorrect in the document.
The top-level heading "Workflow common types and refs" is followed directly by a
third-level heading "ValueInfo" which skips the second-level heading. Fix this
by changing the heading level for ValueInfo from h3 (###) to h2 (##) to create a
proper hierarchical structure that flows from h1 to h2 to h3 as needed.
- Around line 388-392: Remove the blank line that appears between the two
blockquote lines in the common-types-and-refs.md file. The two consecutive
warning blockquotes (both starting with `> ⚠️`) that discuss the "is condition"
and "Select field condition" should be directly adjacent with no blank line
separating them. Delete the empty line at line 390 to comply with Markdownlint
rule MD028 which disallows blank lines within blockquotes.
---
Outside diff comments:
In `@internal/suggest/suggest.go`:
- Around line 55-87: The Closest function now includes containsSegment-based
acceptance and ranking logic, but there are no tests verifying this behavior.
Add test cases for the Closest function that verify the containment-based
acceptance condition (the ct variable), the ranking precedence where containment
takes priority over prefix and distance in the sort logic, and specifically the
example scenario mentioned in the code comment where "+block-list" should be
accepted and ranked higher when searching for similar candidates like
"+base-block-list". Ensure the tests cover all three aspects: acceptance via
containsSegment, ranking precedence in the sort order, and the specific
namespace prefix example.
In `@shortcuts/base/base_dashboard_execute_test.go`:
- Around line 388-394: The error-path test for "invalid data-config json"
currently only checks if an error is returned (err == nil), but does not assert
typed error metadata. After capturing the error from the runShortcut call in the
BaseDashboardBlockCreate test, add assertions using errs.ProblemOf to verify the
error category and subtype, and use errors.As with errs.ValidationError to
assert the Param field. Additionally verify that the error cause is properly
preserved by checking the underlying error chain.
In `@shortcuts/base/workflow_get.go`:
- Around line 33-58: The test coverage for workflow commands (workflow-get,
workflow-enable, and workflow-disable) currently only tests raw token inputs but
does not cover the URL token resolution logic in baseTokenOrRaw() which handles
parsing and resolving /base/ URLs and /wiki/ URLs via API calls. Add test cases
for each of these three workflow commands that exercise the baseTokenOrRaw()
function with URL inputs (both /base/ and /wiki/ URL patterns), verify
successful token resolution from these URLs, and test the error fallback
behavior when URL resolution fails, ensuring all code paths in token handling
are covered by tests.
---
Duplicate comments:
In `@shortcuts/base/view_ops.go`:
- Line 16: Replace all usages of baseTokenOrRaw(runtime) with baseToken(runtime)
throughout the execute paths in view_ops.go (including the Set("base_token",
baseTokenOrRaw(runtime)) call at line 16 and the other occurrences at lines 158,
170, 183, 202, 214, 227, 244, 262, and 278). Since baseToken(runtime) returns
both a value and an error, capture the error return value and return it
immediately from the current function instead of suppressing it with the
fallback behavior, ensuring typed errors are properly propagated rather than
downgraded to raw input values.
---
Nitpick comments:
In `@shortcuts/base/helpers_test.go`:
- Around line 501-538: Replace the error message substring matching using
strings.Contains(err.Error(), ...) in both
TestFieldSearchOptionsKeywordQueryAlias and TestRecordPageSizeAlias test
functions with typed error assertions using errs.ProblemOf to check error
metadata (category/subtype/param) instead. This makes the tests more robust by
verifying the actual error classification rather than relying on message text
that can change, allowing you to assert the specific problem type returned by
the validation methods like BaseFieldSearchOptions.Validate and
BaseRecordList.Validate.
In `@shortcuts/drive/drive_import_test.go`:
- Around line 775-786: The test is using os.Chdir to change the global working
directory, which can cause interference between tests. Remove the origDir, _ :=
os.Getwd() call and the defer os.Chdir(origDir) statement. Instead, use an
absolute path by joining tmpDir with "snapshot.base" in both the os.WriteFile
call and when passing the filename to the mountAndRunDrive function call, so
that the relative path "snapshot.base" is replaced with the full absolute path
throughout this test block.
In `@skills/lark-base/SKILL.md`:
- Line 43: In the Table row description where it currently reads "一张数据表", change
the measure word "张" to "个" to make it "一个数据表" for consistency with conventional
Chinese phrasing for data tables, or alternatively omit the measure word
entirely to read simply "Base 内的数据表" for more natural wording. The current
measure word "张" is typically used for flat objects like sheets or pictures, not
for abstract data structures.
- Line 124: The sentence in line 124 contains a grammatical issue with awkward
repetition of "用户" (user) in the "把" construction. Restructure the text to
improve clarity by removing the redundant "用户" reference. Replace the phrase
"并把用户是否可打开新 Base 告知用户" with a more natural construction that conveys the same
meaning without the double reference to the user, such as using "向用户告知是否可打开新
Base" or a similar alternative that maintains the original intent while
improving grammar.
🪄 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: 04d01395-d7da-460e-a32a-fa861c52b54d
📒 Files selected for processing (134)
internal/suggest/suggest.goshortcuts/base/base_advperm_disable.goshortcuts/base/base_advperm_enable.goshortcuts/base/base_block_create.goshortcuts/base/base_block_delete.goshortcuts/base/base_block_list.goshortcuts/base/base_block_move.goshortcuts/base/base_block_ops.goshortcuts/base/base_block_rename.goshortcuts/base/base_command_common.goshortcuts/base/base_copy.goshortcuts/base/base_dashboard_execute_test.goshortcuts/base/base_data_query.goshortcuts/base/base_dryrun_ops_test.goshortcuts/base/base_execute_test.goshortcuts/base/base_form_create.goshortcuts/base/base_form_delete.goshortcuts/base/base_form_get.goshortcuts/base/base_form_list.goshortcuts/base/base_form_questions_create.goshortcuts/base/base_form_questions_delete.goshortcuts/base/base_form_questions_list.goshortcuts/base/base_form_questions_update.goshortcuts/base/base_form_submit.goshortcuts/base/base_form_update.goshortcuts/base/base_get.goshortcuts/base/base_ops.goshortcuts/base/base_role_create.goshortcuts/base/base_role_delete.goshortcuts/base/base_role_get.goshortcuts/base/base_role_list.goshortcuts/base/base_role_update.goshortcuts/base/base_shortcut_helpers.goshortcuts/base/base_shortcuts_test.goshortcuts/base/dashboard_arrange.goshortcuts/base/dashboard_block_create.goshortcuts/base/dashboard_block_delete.goshortcuts/base/dashboard_block_get.goshortcuts/base/dashboard_block_get_data.goshortcuts/base/dashboard_block_list.goshortcuts/base/dashboard_block_update.goshortcuts/base/dashboard_create.goshortcuts/base/dashboard_delete.goshortcuts/base/dashboard_get.goshortcuts/base/dashboard_list.goshortcuts/base/dashboard_ops.goshortcuts/base/dashboard_update.goshortcuts/base/field_create.goshortcuts/base/field_delete.goshortcuts/base/field_get.goshortcuts/base/field_list.goshortcuts/base/field_list_batch.goshortcuts/base/field_ops.goshortcuts/base/field_search_options.goshortcuts/base/field_update.goshortcuts/base/helpers.goshortcuts/base/helpers_test.goshortcuts/base/record_batch_create.goshortcuts/base/record_batch_update.goshortcuts/base/record_delete.goshortcuts/base/record_get.goshortcuts/base/record_history_list.goshortcuts/base/record_list.goshortcuts/base/record_ops.goshortcuts/base/record_query.goshortcuts/base/record_search.goshortcuts/base/record_share_link_create.goshortcuts/base/record_upload_attachment.goshortcuts/base/record_upsert.goshortcuts/base/shortcuts.goshortcuts/base/table_create.goshortcuts/base/table_delete.goshortcuts/base/table_get.goshortcuts/base/table_list.goshortcuts/base/table_ops.goshortcuts/base/table_update.goshortcuts/base/view_create.goshortcuts/base/view_delete.goshortcuts/base/view_get.goshortcuts/base/view_get_card.goshortcuts/base/view_get_filter.goshortcuts/base/view_get_group.goshortcuts/base/view_get_sort.goshortcuts/base/view_get_timebar.goshortcuts/base/view_get_visible_fields.goshortcuts/base/view_list.goshortcuts/base/view_ops.goshortcuts/base/view_rename.goshortcuts/base/view_set_card.goshortcuts/base/view_set_filter.goshortcuts/base/view_set_group.goshortcuts/base/view_set_sort.goshortcuts/base/view_set_timebar.goshortcuts/base/view_set_visible_fields.goshortcuts/base/workflow_create.goshortcuts/base/workflow_disable.goshortcuts/base/workflow_enable.goshortcuts/base/workflow_execute_test.goshortcuts/base/workflow_get.goshortcuts/base/workflow_list.goshortcuts/base/workflow_update.goshortcuts/drive/drive_import.goshortcuts/drive/drive_import_test.goskills/lark-base/SKILL.mdskills/lark-base/references/formula-examples.mdskills/lark-base/references/formula-field-guide.mdskills/lark-base/references/formula-functions-extended.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-dashboard-block-data-config.mdskills/lark-base/references/lark-base-dashboard-block-get-data.mdskills/lark-base/references/lark-base-dashboard-write.mdskills/lark-base/references/lark-base-dashboard.mdskills/lark-base/references/lark-base-field-json.mdskills/lark-base/references/lark-base-workflow-guide.mdskills/lark-base/references/lark-base-workflow-schema.mdskills/lark-base/references/workflow-steps/action-add-record.mdskills/lark-base/references/workflow-steps/action-delay.mdskills/lark-base/references/workflow-steps/action-find-record.mdskills/lark-base/references/workflow-steps/action-generate-ai-text.mdskills/lark-base/references/workflow-steps/action-http-client.mdskills/lark-base/references/workflow-steps/action-lark-message.mdskills/lark-base/references/workflow-steps/action-set-record.mdskills/lark-base/references/workflow-steps/branch-if-else.mdskills/lark-base/references/workflow-steps/branch-switch.mdskills/lark-base/references/workflow-steps/common-types-and-refs.mdskills/lark-base/references/workflow-steps/system-loop.mdskills/lark-base/references/workflow-steps/trigger-add-record.mdskills/lark-base/references/workflow-steps/trigger-button.mdskills/lark-base/references/workflow-steps/trigger-change-record.mdskills/lark-base/references/workflow-steps/trigger-lark-message.mdskills/lark-base/references/workflow-steps/trigger-reminder.mdskills/lark-base/references/workflow-steps/trigger-set-record.mdskills/lark-base/references/workflow-steps/trigger-timer.mdskills/lark-drive/references/lark-drive-import.md
yballul-bytedance
added a commit
to zhouyue-bytedance/cli
that referenced
this pull request
Jun 23, 2026
Change-Id: I4032881f143183296264b4040eec8361fa297815
yballul-bytedance
force-pushed
the
github-base-token-improve
branch
from
4cd1590 to
6a8bb1a
Compare
github-actions
Bot
added
domain/calendar
yballul-bytedance
force-pushed
the
github-base-token-improve
branch
from
6a8bb1a to
4cd1590
Compare
github-actions
Bot
removed
domain/calendar
Drop overfitted comma-splitting and prefix heuristics, fix workflow trigger schema mismatches, restore deleted high-value workflow content, and clean up dead doc anchors flagged in the PR review. - Remove `normalizePluralReferenceValues` and the hidden `--record-ids` / `--field-names` / `--fields` aliases on `+record-get/list/search`; CSV-style values that contain user data (e.g. "收入,支出对比") are no longer silently split. Negative-path tests assert the aliases are rejected with `unknown flag`. - Remove `isBaseTableID`. `+field-list` / `+field-list-batch` now always resolve refs through `resolveTableRef`, so user table names like `tbl_orders` are no longer mistaken for table IDs. - `executeFieldSearchOptions` now emits only the key the user actually supplied (`field_id` or `field_name`) instead of stuffing the same value into both. Two new tests cover the by-id and by-name paths. - Workflow trigger docs: `trigger-change-record.md` example uses `condition_list` (was `condition`); all four record/reminder triggers call out that `condition_list: []` is rejected by the API and require `null` or omission. `common-types-and-refs.md` uses lowercase `receive_scene = "chat"`. - Workflow guide common-error table now headers itself as a frequency-ordered Top list so agents treat it as the first stop, and picks up four high-frequency rows that were dropped during the PR's doc split (`condition_list: []`, missing `client_token`, `Undefined Step Type`, select-with-text). - Restore the end-to-end IfElseBranch example (trigger → branch → branch actions → join → AI text) into `branch-if-else.md` so agents have a concrete model for `children.links` + `next` wiring. - Test fixtures use the real ID prefixes: dashboard IDs are `blk_*`, chart `block_id`s are `cht_*`. Fixes the dsh_/dash_/blk_a confusion flagged in the review. - Doc cleanup: re-link the orphan 717-line `lark-base-dashboard-block-get-data.md` from `lark-base-dashboard.md`; fix the dead "执行要点" anchor in `SKILL.md` (now points to `lark-base-dashboard-write.md`); add language tags to fenced code blocks (markdownlint MD040) and drop the duplicate `---` rule in `system-loop.md`. Change-Id: I9217be2ee0bc16f53c9462c8a9bd2d69b26f9ea1
Backend already accepts both table IDs and table names on `/bases/:token/tables/:table_id/fields`, so the extra `tables/list` round-trip the client did to translate names to IDs is dead weight — remove `resolveFieldListTableRefs` and the `fieldListTableRef` struct. `+field-list` and `+field-list-batch` now pass `--table-id` straight through, and `+field-list-batch` no longer emits the redundant `table_ref` / `table_name` keys. Tests updated to reflect the simpler request shape (no more tables/list stub) while still covering both ID-only and mixed ID/name inputs. Change-Id: I9abb806144cd23decd0f3453a27e09899f9a491f
…mprove Change-Id: Id765f126612b4acdcc7112954c0435c30057fda5
Remove the unreferenced listEveryTable helper so lint and deadcode gates no longer report unused code. Change-Id: Ieb3e4d103fa37894aa942baa7f8f53e63445feae
Add wiki conditional scope for field-list-batch, strengthen compact and typed-error coverage, and restore a complex workflow guide example. Change-Id: Ie02ec227d6a3b6eefc4853165022f251c8272326
Reword the "complex example" intro to emphasize on-demand reading based on the sketched node set rather than reading all listed files in one go. Convert bare filenames in both example sections to markdown links pointing at workflow-steps/ so readers can navigate to the actual ref files. Change-Id: I085f857aa232d5d7ab215310a78f2d3315dbe5de
yballul-bytedance
force-pushed
the
github-base-token-improve
branch
from
4cd1590 to
1cca939
Compare
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
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.
Supersedes #1353 (recreated from the same branch so pkg.pr.new generates a fresh install command; the previous PR's pinned commit was orphaned by a force-push).
Changes
+field-list-batch, opt-in--compact).--field-names/--fields/--record-ids/--query/--filter/--sort; actionable errors for+record-list --jsonand+data-query --table-id;+dashboard-block-get-dataaccepts--dashboard-id); plural aliases accept repeated values, JSON arrays and comma-separated lists.+block-list->+base-block-list) get suggestions.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.sumdiffRelated Issues
Summary by CodeRabbit
Release Notes
New Features
+field-list-batchto list fields across multiple tables in one call.--compactto+field-listand+field-list-batchfor reduced output.Improvements
drive +import(target-token flow) now includesverification_tokenandverify_hint.Validation & Compatibility
+record-searchsupports deprecated--query(--keyword) and--page-size(--limit), while rejecting conflicting inputs.+record-listrejects deprecated--jsonin favor of--filter-json/--sort-json.+data-queryrejects hidden--table-id;--base-tokenaccepts/base/and/wiki/forms.Documentation