feat(base): auto prune 5 empty rows upon base creation#274
feat(base): auto prune 5 empty rows upon base creation#274Scofy0123 wants to merge 1 commit intolarksuite:mainfrom
Conversation
|
|
github-actions
bot
added
domain/base
|
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:
📝 WalkthroughWalkthroughAdds conditional post-create cleanup that deletes default empty records from a new base's first table unless Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI/User
participant CreateAPI as BaseCreate API
participant TableAPI as Table List API
participant RecordAPI as Record Fetch API
participant DeleteAPI as Record Delete API
CLI->>CreateAPI: create base (keep-empty-rows flag)
CreateAPI-->>CLI: returns base creation data (may include base_token)
alt keep-empty-rows == false
CreateAPI->>TableAPI: list tables using base_token
TableAPI-->>CreateAPI: returns table list
CreateAPI->>RecordAPI: fetch records from first table
RecordAPI-->>CreateAPI: returns record list
loop for each record with empty fields
CreateAPI->>DeleteAPI: DELETE record by id
DeleteAPI-->>CreateAPI: deletion response
end
CreateAPI-->>CreateAPI: aggregate _pruned_records count
end
CreateAPI-->>CLI: output created base data (includes _pruned_records if >0)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds an auto-prune phase to Confidence Score: 4/5Not safe to merge — wrong scope identifier format will cause spurious permission errors for all users of +base-create The prior-round finding about incorrect bitable:* scope strings (correct format is base:* per every other shortcut in this package, e.g. base:table:read and base:record:delete) remains unaddressed and will break the command for all users regardless of --keep-empty-rows. The emptiness-guard fix is a positive improvement, but the scope issue is a P1 that must be resolved first. shortcuts/base/base_create.go — scope string identifiers need correction before merge Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as CLI (executeBaseCreate)
participant LarkAPI as Lark API
CLI->>LarkAPI: POST /open-apis/base/v3/bases
LarkAPI-->>CLI: base data (incl. base_token)
alt --keep-empty-rows is false (default)
CLI->>CLI: extract base_token from response
CLI->>LarkAPI: GET /bases/{base_token}/tables
LarkAPI-->>CLI: tables list
CLI->>CLI: select default table (tables[0])
CLI->>LarkAPI: GET /bases/{base_token}/tables/{table_id}/records?limit=10
LarkAPI-->>CLI: records list
loop for each record where len(fields) == 0
CLI->>LarkAPI: DELETE /bases/{base_token}/tables/{table_id}/records/{record_id}
LarkAPI-->>CLI: delete confirmed
CLI->>CLI: increment deletedTotal
end
end
alt deletedTotal > 0
CLI->>CLI: output {base, created:true, _pruned_records:N}
else
CLI->>CLI: output {base, created:true}
end
Reviews (5): Last reviewed commit: "feat(base): auto prune 5 empty rows upon..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/base/base_create.go`:
- Around line 17-23: The command currently always requests destructive scopes
("bitable:app.table:read" and "bitable:app.table.record:delete") even when the
--keep-empty-rows flag is used; update the scope selection so those scopes are
only added when cleanup is enabled. Modify the code that defines Scopes (and/or
the command bootstrap where executeBaseCreate reads flags) to check the
"keep-empty-rows" flag and append the table-read/record-delete scopes only when
keep-empty-rows is false (or instead perform a separate auth request for cleanup
inside executeBaseCreate when pruning is performed). Ensure references to the
flag name "keep-empty-rows" and the function executeBaseCreate are used to
locate and gate the scope addition.
In `@shortcuts/base/base_ops.go`:
- Around line 108-116: The delete loop currently deletes up to 10 returned
records unconditionally; modify the loop that iterates rawItems (where
baseV3Call is used to fetch records) to call a new or existing predicate
function (e.g., recordLooksEmpty(record map[string]interface{})) and only issue
the DELETE via baseV3Call when that predicate returns true, and enforce a hard
cap to stop after deleting 5 rows; ensure you check item types
(map[string]interface{}) as before and increment a deletedCount only when a
successful delete occurs so the loop exits when deletedCount reaches 5.
🪄 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: 89b2031c-116c-472b-88dd-3aafefac6d5d
📒 Files selected for processing (2)
shortcuts/base/base_create.goshortcuts/base/base_ops.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/base/base_ops.go (1)
126-126: Internal metadata pollutes the API response output.Adding
_pruned_recordsdirectly to thedatamap (which holds the API response) means this internal metadata will appear in the user-facing JSON output at line 133 under"base". Consider storing it at the top level of the output map instead, or omitting it from the final output.♻️ Proposed fix to separate internal metadata from API response
- data["_pruned_records"] = deletedCount + // Store pruned count at output level, not inside API responseThen modify the output at line 133:
output := map[string]interface{}{"base": data, "created": true} if prunedCount, ok := /* track deletedCount separately */; ok && prunedCount > 0 { output["_pruned_records"] = prunedCount } runtime.Out(output, nil)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_ops.go` at line 126, The code currently injects internal metadata into the user-facing response by setting data["_pruned_records"] = deletedCount; remove that assignment so the API payload in the "data" map stays clean, keep tracking deletedCount as a separate local variable, then build a top-level output map (e.g., output := map[string]interface{}{"base": data, "created": true}) and, if deletedCount > 0, add output["_pruned_records"] = deletedCount before calling runtime.Out(output, nil); update any references to "data", "deletedCount", "output", and the runtime.Out(...) call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/base/base_ops.go`:
- Line 126: The code currently injects internal metadata into the user-facing
response by setting data["_pruned_records"] = deletedCount; remove that
assignment so the API payload in the "data" map stays clean, keep tracking
deletedCount as a separate local variable, then build a top-level output map
(e.g., output := map[string]interface{}{"base": data, "created": true}) and, if
deletedCount > 0, add output["_pruned_records"] = deletedCount before calling
runtime.Out(output, nil); update any references to "data", "deletedCount",
"output", and the runtime.Out(...) call accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e747ac8c-b6b0-487e-9b9f-19079c7307b4
📒 Files selected for processing (1)
shortcuts/base/base_ops.go
Scofy0123
force-pushed
the
feat/auto-clean-base-records
branch
from
10f503e to
205145c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/base/base_ops.go (1)
108-109: Consider guarding against empty table ID.If
tableID(tables[0])returns an empty string (e.g., malformed table entry), the API path becomes/bases/.../tables//records, which would fail. While the error is swallowed, adding a guard avoids the unnecessary API call.defaultTableID := tableID(tables[0]) + if defaultTableID == "" { + goto outputResult + } recordsData, err := baseV3Call(runtime, "GET", baseV3Path("bases", baseToken, "tables", defaultTableID, "records"), map[string]interface{}{"limit": 10}, nil)Alternatively, wrap the remaining cleanup logic in
if defaultTableID != "".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_ops.go` around lines 108 - 109, Guard against an empty table ID before calling the API: obtain defaultTableID via tableID(tables[0]) and if it's empty return or skip the baseV3Call to avoid forming a path like "/tables//records"; update the logic around defaultTableID in the function containing baseV3Call/baseV3Path (check defaultTableID != "" and only call baseV3Call or execute the subsequent cleanup when non-empty) so the API call is not made with a malformed path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/base/base_ops.go`:
- Around line 108-109: Guard against an empty table ID before calling the API:
obtain defaultTableID via tableID(tables[0]) and if it's empty return or skip
the baseV3Call to avoid forming a path like "/tables//records"; update the logic
around defaultTableID in the function containing baseV3Call/baseV3Path (check
defaultTableID != "" and only call baseV3Call or execute the subsequent cleanup
when non-empty) so the API call is not made with a malformed path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cde45eb1-1fe9-46f4-8257-bdac4b1153f2
📒 Files selected for processing (2)
shortcuts/base/base_create.goshortcuts/base/base_ops.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/base/base_create.go
- Prunes the 5 empty rows automatically generated by Feishu during base creation. - Retains rows if --keep-empty-rows is specified. - Adds emptiness guard to prevent deleting user records during prune. - Ensures API response structure is unmutated.
Scofy0123
force-pushed
the
feat/auto-clean-base-records
branch
from
205145c to
ead5ec5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shortcuts/base/base_ops.go (1)
112-125:⚠️ Potential issue | 🟠 MajorEnforce a hard stop after deleting 5 rows.
Line 112 iterates all fetched records (limit 10), and deletions continue as long as rows look empty. This can prune more than the intended default five rows.
💡 Proposed fix
if rawItems, ok := recordsData["items"].([]interface{}); ok { for _, item := range rawItems { + if deletedTotal >= 5 { + break + } if recMap, ok := item.(map[string]interface{}); ok { // Emptiness guard: only delete if 'fields' is strictly empty (no values filled yet) fieldsMap, _ := recMap["fields"].(map[string]interface{}) if len(fieldsMap) == 0 { if recID, ok := recMap["record_id"].(string); ok { _, delErr := baseV3Call(runtime, "DELETE", baseV3Path("bases", baseToken, "tables", defaultTableID, "records", recID), nil, nil) if delErr == nil { deletedTotal++ } } } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/base_ops.go` around lines 112 - 125, The loop over rawItems in base_ops.go can delete more than five rows; enforce a hard cap by checking deletedTotal against the limit (5) and breaking out once reached: inside the loop that iterates rawItems (the block that checks fieldsMap and calls baseV3Call to delete a record), add a guard so if deletedTotal >= 5 you stop further deletions (break out of the loop or return from the enclosing function as appropriate), ensuring the increment of deletedTotal remains where it is after a successful delete; reference symbols: rawItems, deletedTotal, fieldsMap, baseV3Call, baseV3Path, defaultTableID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/base/base_ops.go`:
- Around line 115-117: The current guard uses a failed type assertion
(fieldsMap, _ := recMap["fields"].(map[string]interface{})) so nil is treated as
empty; change the check to require a successful assertion and only treat truly
empty maps as deletable—i.e., perform fieldsMap, ok :=
recMap["fields"].(map[string]interface{}) and only enter the deletion branch
when ok is true and len(fieldsMap) == 0; reference the existing recMap,
fieldsMap and the deletion branch that uses recID/record_id to locate the
record.
---
Duplicate comments:
In `@shortcuts/base/base_ops.go`:
- Around line 112-125: The loop over rawItems in base_ops.go can delete more
than five rows; enforce a hard cap by checking deletedTotal against the limit
(5) and breaking out once reached: inside the loop that iterates rawItems (the
block that checks fieldsMap and calls baseV3Call to delete a record), add a
guard so if deletedTotal >= 5 you stop further deletions (break out of the loop
or return from the enclosing function as appropriate), ensuring the increment of
deletedTotal remains where it is after a successful delete; reference symbols:
rawItems, deletedTotal, fieldsMap, baseV3Call, baseV3Path, defaultTableID.
🪄 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: fcab5366-91c4-4af6-844d-7be01b004917
📒 Files selected for processing (2)
shortcuts/base/base_create.goshortcuts/base/base_ops.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/base/base_create.go
|
感谢您的建议。我们正在研究更优的解法，准备从接口侧支持这个需求，有进展会及时同步出来。 Thanks for the feedback. We are working on a better approach and plan to support this at the API level. We will share updates promptly as progress is made. |
3 participants
Motivation / 背景动因
In scenarios where
lark-cliis utilized within an automated pipeline (e.g., CI/CD, data scraping, syncing scripts), creating a new Base is explicitly intended for programmatic data ingestion. However, the Feishu OpenAPI inherently initializes 5 empty records in the default table of a newly created Base.While this is helpful for human users via the GUI to understand where to input data, it introduces significant side-effects and deterministic data order issues for scripts and developers writing to an unexpectedly "dirty" table.
在自动化的工作流场景中（如爬虫同步、数据自动化录入等），开发者通过
lark-cli创建一个全新的多维表格（Base），其唯一目的就是提供一个结构化的、干净的数据底座以便后续程序化定点写入数据。然而，受限于飞书平台层的默认行为，新建的 Base 内部第一张默认表会自动生成 5 行空数据。这对机器脚本而言是非常破坏性的（会导致新插数据行号偏移或数据完整层面的错乱）。Proposed Solution / 解决方案
This PR implements an orchestrating "cleanup" phase internally attached to the
executeBaseCreateworkflow:--keep-empty-rowsboolean flag (defaults tofalse), maintaining zero-config convenience while pruning records by default.bitable:app.table:read&bitable:app.table.record:delete) into the base create command definitions to ensure seamless permissions.createcommand remains successful regardless of unexpected permission restrictions.本 PR 包含以下优化，主要是在
executeBaseCreate中挂载了尽力而为的静默后置清理操作：lark-cli base +base-create命令追加--keep-empty-rows退出选项（默认false，即遵循“机器建表必须干净”的潜规则）。POST) 成功且返回 Token 后，利用已封装完成的底层方法，找到产生的首张表执行GET records后遍历DELETE以实现秒级“自清理”。Type:
feat(DX improvement)Summary by CodeRabbit
New Features
keep-empty-rowsflag (default: false) to control whether default placeholder rows are preserved after creating a new base.Bug Fixes / Improvements