fix(base): harden agent recovery for fields and views - #2153
Open
huarenmin13 wants to merge 6 commits into
Open
Conversation
1. Reject unknown field types before dry-run or execution with typed recovery guidance.
2. Normalize created view IDs and fail closed when the API omits creation evidence.
3. Add scoped hidden name aliases and deduplicated table-copy and view guidance with regression cove
rage.
```ai-signature
改动范围: Base field and view shortcuts, scoped local aliases, skill references, and unit and dry-run tests.
思考过程: Compared the retained purposes from both internal MRs, kept complementary runtime behavior, split long guidance by workflow, and removed reverted, unvalidated, and duplicate prose-contract changes.
改动原因: Agents need deterministic validation, explicit creation evidence, and narrowly scoped recovery paths instead of guessing unsupported fields, reusing same-name views, or expanding aliases globally.
Break Change: 否
```
Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local>
AI-SHA256: 29b2a119a0822d5af6eaa70cc885e8d19144e48372f5acb6f6305a3cc097fddf
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:
📝 WalkthroughWalkthroughThe change adds canonical field-type validation, hidden local resource-name aliases, stricter view-creation result handling, batch partial-failure reporting, updated view-management guidance, and unit and end-to-end tests. ChangesBase capability updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant BaseViewCreate
participant ViewAPI
participant OutputEnvelope
CLI->>BaseViewCreate: submit view creation batch
BaseViewCreate->>ViewAPI: create each view
ViewAPI-->>BaseViewCreate: return id or view_id
BaseViewCreate->>OutputEnvelope: normalize successful identifiers
ViewAPI-->>BaseViewCreate: return later item failure
BaseViewCreate-->>CLI: return successes and partial-failure details
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/base/base_shortcuts_test.go`:
- Line 24: Remove the direct internal/vfs import from the shortcut test and
replace the vfs.ReadFile usage in the relevant test with the test’s existing
file I/O mechanism; only modify the shortcuts-no-vfs depguard rule if that
dependency is intentional.
🪄 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 Plus
Run ID: ed82df0c-7f08-4afd-bea9-66091bb21d10
📒 Files selected for processing (17)
shortcuts/base/base_execute_test.goshortcuts/base/base_shortcuts_test.goshortcuts/base/field_get.goshortcuts/base/field_ops.goshortcuts/base/local_flag_alias.goshortcuts/base/table_get.goshortcuts/base/view_create.goshortcuts/base/view_list.goshortcuts/base/view_ops.goshortcuts/base/view_set_visible_fields.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-table-copy.mdskills/lark-base/references/lark-base-view-create.mdtests/cli_e2e/base/base_field_dryrun_test.gotests/cli_e2e/base/base_field_update_dryrun_test.gotests/cli_e2e/base/base_resource_name_alias_dryrun_test.gotests/cli_e2e/base/base_view_contract_dryrun_test.go
1. Remove the same-Base single-table copy route from the lark-base skill. 2. Delete the dedicated table-copy SOP while preserving independent field, view, and alias changes. ```ai-signature 改动范围: Base skill routing and the single-table copy reference introduced in PR larksuite#2153. 思考过程: Removed the complete single-table-copy purpose group while preserving independent field validation, view creation, and local alias behavior and tests. 改动原因: The reviewer requested that same-Base single-table copy guidance be excluded from this pull request. Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 40e5a1fcd3f006bb4c7102bb972c3ca5e9de3924f8f1fbaf53b7ba391cf19657
1. 统一字段类型目录并覆盖 field、table、base 的全部字段写入入口 2. 拒绝空视图批次，规范新建 ID，并在批量后项失败时保留已创建结果 3. 移除视图 UI 能力边界改动，保留隐藏资源名别名的最小恢复路径 4. 补充单元和编译后二阶段 dry-run 回归 ```ai-signature 改动范围: Base 字段创建和更新校验、Base 与表创建字段数组、视图创建部分失败输出、隐藏资源名别名、Skill 路由与单元及 dry-run 测试。 思考过程: 按改动目的删除 base_table_043 的 UI 能力边界整组内容；字段类型复用单一目录覆盖所有写入口，视图批量失败复用已有部分失败协议，别名不进入 Shortcut 元数据且不扩大可见帮助。 改动原因: 现有 PR 只覆盖部分字段入口且批量视图失败会丢失已创建 ID，容易让 Agent 误报或重复写入；同时用户要求移除题目特化的 UI 边界内容并将剩余修复收敛为通用最小实现。 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 3fc8cee5c6e8f512794bfd97ecac862ca9a4a8c759c63076c93179f9d4ab1564
1. 使用标准库 os.ReadFile 读取字段类型参考文档 2. 保留字段类型目录与文档一致性的原有断言 ```ai-signature 改动范围: shortcuts/base/base_shortcuts_test.go 的字段类型目录一致性测试及其 import。 思考过程: CodeRabbit 指出的 shortcuts 到 internal/vfs 依赖边界问题在当前代码中仍存在；测试只需读取静态文档，标准库 os.ReadFile 已满足需求，无需修改 depguard 规则或生产代码。 改动原因: 移除不必要的内部包依赖，使快捷命令测试遵守 shortcuts-no-vfs 边界，同时保持字段类型 SSOT 回归覆盖不变。 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 0597f6f205e613fe9f16ec945af102647792886a1b04c68b927180d71511587b
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/base/view_ops.go (1)
200-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the repeated partial-failure branch.
Line 200-201 and Line 209-210 repeat the same
if len(created) > 0 { return viewCreateProgressError(runtime, err, created, index+1) }pattern. Extracting a small helper, e.g.failOrProgress(err), would remove the duplication and reduce the chance the two branches drift apart later.♻️ Proposed refactor to consolidate the duplicated branch
+ fail := func(err error) error { + if len(created) > 0 { + return viewCreateProgressError(runtime, err, created, index+1) + } + return err + } for index, body := range viewItems { data, err := baseV3Call(runtime, "POST", baseV3Path("bases", baseToken, "tables", tableIDValue, "views"), nil, body) if err != nil { - if len(created) > 0 { - return viewCreateProgressError(runtime, err, created, index+1) - } - return err + return fail(err) } newViewID := viewID(data) if newViewID == "" { err := errs.NewValidationError(errs.SubtypeFailedPrecondition, "view create response omitted the new view ID; creation state is unknown"). WithHint("Do not retry or configure a same-name view. If verification is required and read permission is available, run +view-list to reconcile the view IDs.") - if len(created) > 0 { - return viewCreateProgressError(runtime, err, created, index+1) - } - return err + return fail(err) }Also applies to: 209-210
🤖 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` around lines 200 - 201, Extract the repeated partial-failure handling around view creation into a small local helper, such as failOrProgress, that checks len(created) and returns viewCreateProgressError(runtime, err, created, index+1) when applicable. Replace both duplicated branches near the view creation logic with this helper while preserving their existing error behavior and control flow.
🤖 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 `@tests/cli_e2e/base/base_create_dryrun_test.go`:
- Line 136: Update the assertions at
tests/cli_e2e/base/base_create_dryrun_test.go:136 and
tests/cli_e2e/base/base_table_create_dryrun_test.go:25 to parse stderr with
gjson and assert the expected text against the decoded error.message field,
rather than matching a raw JSON substring.
---
Nitpick comments:
In `@shortcuts/base/view_ops.go`:
- Around line 200-201: Extract the repeated partial-failure handling around view
creation into a small local helper, such as failOrProgress, that checks
len(created) and returns viewCreateProgressError(runtime, err, created, index+1)
when applicable. Replace both duplicated branches near the view creation logic
with this helper while preserving their existing error behavior and control
flow.
🪄 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 Plus
Run ID: 6208717d-f016-4cf3-9a00-2bda8236b791
📒 Files selected for processing (12)
shortcuts/base/base_execute_test.goshortcuts/base/base_ops.goshortcuts/base/base_shortcuts_test.goshortcuts/base/field_ops.goshortcuts/base/local_flag_alias.goshortcuts/base/table_ops.goshortcuts/base/view_ops.goskills/lark-base/SKILL.mdskills/lark-base/references/lark-base-view-create.mdtests/cli_e2e/base/base_create_dryrun_test.gotests/cli_e2e/base/base_table_create_dryrun_test.gotests/cli_e2e/base/base_view_contract_dryrun_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- shortcuts/base/local_flag_alias.go
- skills/lark-base/SKILL.md
- shortcuts/base/field_ops.go
- skills/lark-base/references/lark-base-view-create.md
- shortcuts/base/base_execute_test.go
- shortcuts/base/base_shortcuts_test.go
1. 从 stderr 的 error.message 读取 Base 创建字段类型错误 2. 对表创建字段类型错误使用相同结构化断言 ```ai-signature 改动范围: Base 与表创建 dry-run 的未知字段类型 E2E 断言，共两个测试文件。 思考过程: 原断言直接在完整 stderr JSON 上匹配转义字符串，可能误命中其他字段；改为使用已有 gjson 读取 error.message，精确锁定本次校验契约且不改变生产代码。 改动原因: CodeRabbit 指出的结构化断言问题有效，修复能让测试直接验证错误消息字段并避免原始 JSON 串匹配造成的假阳性。 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 467afe0a49196831f9ae2965914cad2bcee1e67344376613916e811b4454959a
1. 将视图创建规则合并到现有 lark-base Skill，删除独立短 reference 2. 复用 pflag 解析期归一化实现资源名别名，删除额外冲突和空值分支 3. 把字段目录一致性校验移到现有 Skill 契约测试，并同步 E2E 文件名与覆盖清单 ```ai-signature 改动范围: Base shortcut 通用 helper、三个资源读取 shortcut、lark-base Skill、字段与视图契约测试及 E2E 覆盖清单。 思考过程: 优先复用仓库中 Sheets 和 Slides 已采用的 pflag normalize 机制，并把只服务 Skill 文档的校验放回现有 Skill contract 测试，避免为短规则和局部别名各建独立文件。 改动原因: 当前 PR 中新增的短 view reference 和 alias helper 落点偏重，且冲突与空值处理超出目标问题；本次按通用且最小原则删除额外层次和单题化行为。 Break Change: 否 ``` Co-authored-by: BASE Infra Harness <ai@base-infra-harness.noreply.local> AI-SHA256: 3ea09e66102ca1442d21d17f265398fbe93e64edee66739deafc94ac0b131cc1
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
1 participant
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
Harden Base shortcuts around three reusable contracts: canonical field types, concrete view-creation evidence, and parse-time recovery aliases for common resource-name flags. This combines the retained purposes from the two evaluated internal MRs without changing the reference PR #2133.
The
base_table_043view UI capability-boundary changes and the same-Base single-table-copy guidance are intentionally excluded from this PR.Changes
+field-create,+field-update,+table-create --fields, and+base-create --fields; malformed field arrays and unsupported types fail before dry-run or execution.+view-createitem, normalize alternateview_idresponses toviews[].id, and fail closed when a create response has no ID.ok:falsepartial-failure contract when a later view in a batch fails, preserving earlierviewsand structured failure metadata so callers do not repeat successful writes.skills/lark-base/SKILL.md; no separate leaf reference is added.Generic and minimal rationale
OutPartialFailureprotocol instead of adding a new output or exit mechanism.Test Plan
go test ./shortcuts/base -count=1go vet ./shortcuts/base ./tests/cli_e2e/basegit diff --checkRelated Issues
Co-authored-by: BASE Infra Harness ai@base-infra-harness.noreply.local
AI-SHA256: 69994e6e64df828d727826fea9ad275028c4c705609ad1399e881ba524118418