docs(base): refine record cell value guidance#636
Conversation
1. Reject top-level fields wrappers in base +record-upsert input and keep request bodies as field maps. 2. Replace record-upsert tests with Map<FieldNameOrID, CellValue> input and assert the outgoing body has no fields wrapper. 3. Consolidate Base record value documentation around lark-base-cell-value and update record command references.
|
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 replaces the old Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 |
1. Remove the dedicated record-upsert parser and restore the shared record JSON object validation path. 2. Keep record-upsert dry-run and execution as raw JSON object passthrough. 3. Drop the test assertion that rejected a top-level fields key for record-upsert.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@07a97033dc10f8fb82c5c2fa446a55e0f954607f🧩 Skill updatenpx skills add zgz2048/cli#codex/refactor-base-cell-docs -y -g |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
shortcuts/base/base_shortcuts_test.go (1)
255-270: Positive-case coverage for field-map input LGTM.The added assertion validates the new top-level
Map<FieldNameOrID, CellValue>contract for+record-upsert. Consider also adding a negative case asserting that a{"fields": {...}}wrapper is rejected (or explicitly accepted as raw JSON per the second commit) to lock in the documented behavior — the PR description mentions the rejection test was later dropped, so a clear positive/negative boundary test would prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_shortcuts_test.go` around lines 255 - 270, Add a negative test that ensures the top-level Map<FieldNameOrID, CellValue> contract is enforced for +record-upsert by calling BaseRecordUpsert.Validate(ctx, newBaseTestRuntime(..., map[string]string{"base-token":"b","table-id":"tbl_1","json":`{"fields":{"Name":"Alice"}}`}, nil, nil)) and asserting it returns a non-nil error; place it near the existing positive-case that uses BaseRecordUpsert.Validate so the suite explicitly verifies that the wrapped {"fields": {...}} form is rejected (use the same ctx and test helper newBaseTestRuntime and check err != nil).skills/lark-base/references/lark-base-record-share-link-create.md (1)
12-20: Inconsistent placeholder style within the multi-record example.On line 20,
--record-ids rec001,rec002,rec003still uses concrete-looking IDs while the surrounding--base-token/--table-idwere normalized to<base_token>/<table_id>. Consider<record_id_1>,<record_id_2>,<record_id_3>for consistency with the rest of the PR's placeholder convention. Minor nit — feel free to skip if the example IDs were kept intentionally to illustrate comma-separated format.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/references/lark-base-record-share-link-create.md` around lines 12 - 20, Update the multi-record example for the lark-cli base +record-share-link-create command to use placeholder-style record IDs for consistency: replace the concrete-looking "rec001,rec002,rec003" after the --record-ids flag with "<record_id_1>,<record_id_2>,<record_id_3>" so it matches the surrounding "<base_token>" and "<table_id>" placeholders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-base/references/lark-base-cell-value.md`:
- Around line 23-29: The example JSON in lark-base-cell-value.md uses an invalid
13-digit phone number ("1380000000000"); update the sample to a valid 11-digit
Chinese mobile number (e.g., "13800000000") so downstream agents won't consume
malformed phone data—locate the JSON block in the file and replace the value for
the "联系电话" field accordingly.
- Around line 86-96: Update the "2.7 link" example so the link-field CellValue
uses the correct object shape: replace the array-of-objects example (e.g., [{
"id": "<record_id>" }]) with an object whose key is "link_record_ids" and value
is an array of record_id strings (e.g., { "link_record_ids": ["rec_xxx"] });
ensure both the read and write examples in the "2.7 link" section show this
exact structure and mention that two-way link fields use the same
"link_record_ids" format.
---
Nitpick comments:
In `@shortcuts/base/base_shortcuts_test.go`:
- Around line 255-270: Add a negative test that ensures the top-level
Map<FieldNameOrID, CellValue> contract is enforced for +record-upsert by calling
BaseRecordUpsert.Validate(ctx, newBaseTestRuntime(...,
map[string]string{"base-token":"b","table-id":"tbl_1","json":`{"fields":{"Name":"Alice"}}`},
nil, nil)) and asserting it returns a non-nil error; place it near the existing
positive-case that uses BaseRecordUpsert.Validate so the suite explicitly
verifies that the wrapped {"fields": {...}} form is rejected (use the same ctx
and test helper newBaseTestRuntime and check err != nil).
In `@skills/lark-base/references/lark-base-record-share-link-create.md`:
- Around line 12-20: Update the multi-record example for the lark-cli base
+record-share-link-create command to use placeholder-style record IDs for
consistency: replace the concrete-looking "rec001,rec002,rec003" after the
--record-ids flag with "<record_id_1>,<record_id_2>,<record_id_3>" so it matches
the surrounding "<base_token>" and "<table_id>" placeholders.
🪄 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: 37331722-92bc-484f-9315-84a47f6de61d
📒 Files selected for processing (24)
shortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/helpers.goshortcuts/base/helpers_test.goshortcuts/base/record_batch_create.goshortcuts/base/record_batch_update.goshortcuts/base/record_upsert.goskill-template/domains/base.mdskills/lark-base/SKILL.mdskills/lark-base/references/examples.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-data-query.mdskills/lark-base/references/lark-base-record-batch-create.mdskills/lark-base/references/lark-base-record-batch-update.mdskills/lark-base/references/lark-base-record-delete.mdskills/lark-base/references/lark-base-record-get.mdskills/lark-base/references/lark-base-record-history-list.mdskills/lark-base/references/lark-base-record-list.mdskills/lark-base/references/lark-base-record-search.mdskills/lark-base/references/lark-base-record-share-link-create.mdskills/lark-base/references/lark-base-record-upload-attachment.mdskills/lark-base/references/lark-base-record-upsert.mdskills/lark-base/references/lark-base-record.mdskills/lark-base/references/lark-base-shortcut-record-value.md
💤 Files with no reviewable changes (2)
- skills/lark-base/references/lark-base-shortcut-record-value.md
- shortcuts/base/helpers.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #636 +/- ##
==========================================
+ Coverage 59.96% 60.65% +0.68%
==========================================
Files 405 416 +11
Lines 42710 43969 +1259
==========================================
+ Hits 25611 26669 +1058
- Misses 15088 15231 +143
- Partials 2011 2069 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/lark-base/references/lark-base-record-batch-create.md`:
- Around line 50-54: The "+record-batch-update" documentation is missing the
select-option auto-creation warning present in "+record-batch-create" and
"+record-upsert"; update the "坑点" section of the record-batch-update doc to add
this exact warning line: "select 写入未知选项时平台可能自动新增选项;如果不是要新增选项,先确认真实选项名。" so that
the `patch` field (which can contain select cell values) is covered consistently
with `+record-batch-create` and `+record-upsert`.
🪄 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: fb116b55-9a69-46cf-95d9-db70b598af59
📒 Files selected for processing (5)
skills/lark-base/SKILL.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-record-batch-create.mdskills/lark-base/references/lark-base-record-batch-update.mdskills/lark-base/references/lark-base-record-upsert.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-base/references/lark-base-cell-value.md
- skills/lark-base/references/lark-base-record-batch-update.md
1520075 to
e9735e3
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/lark-base/references/lark-base-record-batch-update.md (1)
45-49:⚠️ Potential issue | 🟡 Minor补充 select 未知选项自动新增的风险提示。
当前“坑点”缺少与其他 record 写入文档一致的提示:
patch写入 select 字段时,未知选项可能被平台自动新增,容易造成脏数据。建议在 Line 45-Line 49 增加同款警示语,保证三份文档口径一致。📌 建议补丁
## 坑点 - 这是“同值批量更新”:所有 `record_id_list` 都应用同一份 `patch`。 - `record_id_list` 最大 200 条,超过会被接口校验拒绝。 - 命令不会自动做字段/行映射转换,传什么就发什么。 - 如果 `patch` 包含只读字段,返回里可能出现 `ignored_fields`;这些字段不会被更新。 +- select 写入未知选项时平台可能自动新增选项;如果不是要新增选项,先确认真实选项名。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/references/lark-base-record-batch-update.md` around lines 45 - 49, Add a warning in the "同值批量更新" section explaining that when a patch writes to select-type fields, unknown options may be automatically created by the platform (causing dirty data), and recommend validating select values before sending patches; update the text near the bullets referencing record_id_list and patch so it matches the same cautionary wording used in other record write docs (mention select fields, unknown options auto-added, and potential data pollution).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@skills/lark-base/references/lark-base-record-batch-update.md`:
- Around line 45-49: Add a warning in the "同值批量更新" section explaining that when
a patch writes to select-type fields, unknown options may be automatically
created by the platform (causing dirty data), and recommend validating select
values before sending patches; update the text near the bullets referencing
record_id_list and patch so it matches the same cautionary wording used in other
record write docs (mention select fields, unknown options auto-added, and
potential data pollution).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59092bf6-088c-4649-a8f3-5c5efe888852
📒 Files selected for processing (6)
skill-template/domains/base.mdskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-record-batch-create.mdskills/lark-base/references/lark-base-record-batch-update.mdskills/lark-base/references/lark-base-record-upsert.md
🚧 Files skipped from review as they are similar to previous changes (3)
- skills/lark-base/references/lark-base-cell-value.md
- skill-template/domains/base.md
- skills/lark-base/references/lark-base-record-upsert.md
e9735e3 to
a2b1f29
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
skills/lark-base/references/lark-base-record-batch-update.md (1)
45-49:⚠️ Potential issue | 🟡 MinorAdd select unknown-option warning here for parity with other record-write docs.
patchcan include select CellValue, but this pitfalls section omits the auto-create warning already documented in sibling pages. Please re-add the same caution for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/references/lark-base-record-batch-update.md` around lines 45 - 49, The pitfall section for the "同值批量更新" documentation is missing the select unknown-option auto-create warning; update the paragraph that describes `patch` and select CellValue to include the same caution used in sibling record-write docs: explicitly note that if `patch` contains a select option not present in the schema it may be auto-created (or rejected depending on tenant settings), and advise callers to validate select option existence before sending updates; reference `patch`, `select CellValue`, `record_id_list`, and `ignored_fields` when adding the warning for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skill-template/domains/base.md`:
- Line 13: The doc contains an inconsistent batch-size statement: change the
line that currently reads "6. **批量上限 500 条/次**" to match the rest of the
document and base record docs by using "200 条/次" (i.e., 200-row limit); update
any nearby phrasing that references "500" (for example the sentence at Line 13
and any duplicate mention around Line 110) so all occurrences and bullet text
reflect the 200-row batch limit.
---
Duplicate comments:
In `@skills/lark-base/references/lark-base-record-batch-update.md`:
- Around line 45-49: The pitfall section for the "同值批量更新" documentation is
missing the select unknown-option auto-create warning; update the paragraph that
describes `patch` and select CellValue to include the same caution used in
sibling record-write docs: explicitly note that if `patch` contains a select
option not present in the schema it may be auto-created (or rejected depending
on tenant settings), and advise callers to validate select option existence
before sending updates; reference `patch`, `select CellValue`, `record_id_list`,
and `ignored_fields` when adding the warning for consistency.
🪄 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: 90cbdb78-e110-4c25-8235-8097b876bc1f
📒 Files selected for processing (6)
skill-template/domains/base.mdskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-record-batch-create.mdskills/lark-base/references/lark-base-record-batch-update.mdskills/lark-base/references/lark-base-record-upsert.md
✅ Files skipped from review due to trivial changes (1)
- skills/lark-base/references/lark-base-cell-value.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-base/references/lark-base-record-upsert.md
1. Align record CellValue examples with live behavior for date, URL, user, link, select, numeric styles, and readonly fields. 2. Remove misleading user_id_type and execution identity prompts from record-writing guidance. 3. Keep record JSON file input guidance generic and avoid documenting environment-specific stdin or path limits.
a2b1f29 to
07a9703
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
skills/lark-base/SKILL.md (1)
330-330: LGTM! The guidance clarifies readonly field handling.The updated error recovery instruction clearly states that readonly fields (system fields, formula, lookup) should be removed, and only storage fields should be written. This aligns with the tightened
ignored_fieldsmessaging across the PR.Note: Static analysis suggests a minor grammar refinement ("可被……当成" instead of "被当成"), but the current phrasing is clear and acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-base/SKILL.md` at line 330, Update the wording for the `ignored_fields` / `READONLY` table row: change the phrase "只读字段被当成可写字段" to the grammatically refined "只读字段可被当成可写字段" (or similar "只读字段可被当成可写字段,常见于系统字段、formula、lookup") while keeping the rest of the guidance ("移除只读字段,只写存储字段;计算结果交给 formula / lookup / 系统字段自动产出") unchanged so the meaning remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@skills/lark-base/SKILL.md`:
- Line 330: Update the wording for the `ignored_fields` / `READONLY` table row:
change the phrase "只读字段被当成可写字段" to the grammatically refined "只读字段可被当成可写字段" (or
similar "只读字段可被当成可写字段,常见于系统字段、formula、lookup") while keeping the rest of the
guidance ("移除只读字段,只写存储字段;计算结果交给 formula / lookup / 系统字段自动产出") unchanged so the
meaning remains consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39d29355-cfee-482f-94bb-d87e8b6907f4
📒 Files selected for processing (6)
skill-template/domains/base.mdskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-cell-value.mdskills/lark-base/references/lark-base-record-batch-create.mdskills/lark-base/references/lark-base-record-batch-update.mdskills/lark-base/references/lark-base-record-upsert.md
✅ Files skipped from review due to trivial changes (2)
- skills/lark-base/references/lark-base-cell-value.md
- skills/lark-base/references/lark-base-record-batch-update.md
🚧 Files skipped from review as they are similar to previous changes (2)
- skills/lark-base/references/lark-base-record-upsert.md
- skills/lark-base/references/lark-base-record-batch-create.md
Summary
Consolidate Base record write guidance around
lark-base-cell-value.mdas the source of truth for+record-upsert,+record-batch-create, and+record-batch-update.The CLI keeps using the shared JSON object parsing path for
+record-upsert; this PR updates help/docs/tests to prefer top-levelMap<FieldNameOrID, CellValue>inputs and removes stale{fields}wrapper guidance from the upsert path.Changes
lark-base-cell-value.mdand update record command references to reuse it.+record-upserthelp text and tests to use top-level field-map input.{fields}.Test Plan
go test ./shortcuts/basemake unit-testmake buildAgent Regression
./lark-cli version v1.0.12-62-g0c2e3b4../lark-cliand currentskills/lark-basedocs only; they did not read old global skills or source code.+record-upload-attachment.ignored_fields/READONLYinstead of being updated, datetime CellValue acceptsYYYY-MM-DD HH:mm:ss, and stdin is not documented as a supported interface.not_foundwith available-field hints, deleting the last table is blocked, and concurrent Base creation can hit API method limiting.Related Issues
Summary by CodeRabbit
Documentation
Tests