feat(docs): support block mutation ranges - #2265
Conversation
📝 WalkthroughWalkthroughThe docs update command now supports inclusive multi-block ChangesInclusive block mutation ranges
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant docs_update_v2
participant DocumentsEndpoint
participant DocumentFetch
CLI->>docs_update_v2: submit ranged block_replace or block_delete
docs_update_v2->>docs_update_v2: validate range endpoints
docs_update_v2->>DocumentsEndpoint: send mutation request
DocumentsEndpoint-->>CLI: return update result
CLI->>DocumentFetch: fetch document content
DocumentFetch-->>CLI: return updated content
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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/ccm
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2265 +/- ##
==========================================
+ Coverage 76.36% 76.44% +0.08%
==========================================
Files 1011 1025 +14
Lines 111269 113732 +2463
==========================================
+ Hits 84970 86945 +1975
- Misses 19815 20113 +298
- Partials 6484 6674 +190 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/doc/docs_update_v2.go`:
- Around line 76-79: Stop silently trimming block IDs: in
shortcuts/doc/docs_update_v2.go:76-79, preserve the supplied startBlockID and
endBlockID values or reject whitespace-padded IDs with the existing typed
validation pattern; in shortcuts/doc/docs_update_v2.go:276-280, serialize those
exact endpoint values without strings.TrimSpace; update
shortcuts/doc/docs_update_test.go:223-247 to verify exact preservation or typed
rejection instead of expecting trimmed values.
In `@tests/cli_e2e/docs/docs_update_dryrun_test.go`:
- Around line 102-119: Add a live bot-credential E2E test alongside the existing
block_replace coverage that creates a document containing three blocks, replaces
the inclusive range from li1 through li3, fetches the document, and asserts the
resulting block content. Make the test self-contained with document cleanup
using the existing test helpers and conventions.
🪄 Autofix
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: e2165043-8667-4996-be8a-27ce9c7c65cb
📒 Files selected for processing (4)
shortcuts/doc/docs_update_test.goshortcuts/doc/docs_update_v2.goskills/lark-doc/references/lark-doc-update.mdtests/cli_e2e/docs/docs_update_dryrun_test.go
| { | ||
| name: "block_replace inclusive range", | ||
| args: []string{ | ||
| "docs", "+update", | ||
| "--doc", "doxcnDryRunE2E", | ||
| "--command", "block_replace", | ||
| "--start-block-id", "li1", | ||
| "--end-block-id", "li3", | ||
| "--content", "<li>combined</li>", | ||
| "--dry-run", | ||
| }, | ||
| wantContains: []string{"/open-apis/docs_ai/v1/documents/doxcnDryRunE2E"}, | ||
| wantBody: map[string]any{ | ||
| "command": "block_replace", | ||
| "start_block_id": "li1", | ||
| "end_block_id": "li3", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add live E2E coverage for inclusive range replacement.
This dry-run case verifies only request serialization. It does not verify that the service replaces every block from start_block_id through end_block_id.
Add a live bot-credential test that creates a document with three blocks, replaces the inclusive range, fetches the document, asserts the resulting block content, and cleans up the document.
As per coding guidelines, “New flows or behavior changes require live E2E coverage with a self-contained create/use/cleanup workflow and bot credentials where applicable.”
🤖 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 `@tests/cli_e2e/docs/docs_update_dryrun_test.go` around lines 102 - 119, Add a
live bot-credential E2E test alongside the existing block_replace coverage that
creates a document containing three blocks, replaces the inclusive range from
li1 through li3, fetches the document, and asserts the resulting block content.
Make the test self-contained with document cleanup using the existing test
helpers and conventions.
Source: Coding guidelines
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@baec82bcd8812a56889804c57368d908707b6a63🧩 Skill updatenpx skills add larksuite/cli#codex/block-replace-range -y -g |
SunPeiYang996
changed the title
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 `@tests/cli_e2e/docs/docs_update_test.go`:
- Around line 97-101: Add a distinct trailing fixture block after the final
entry in the deletion range used by the test, then assert after the delete
operation that this trailing block still exists. Update the nearby assertions in
the test’s deletion flow so the regression specifically verifies blocks after
end-block-id are preserved.
🪄 Autofix
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: 7ead8433-1e6f-42bc-9645-16944dbb1e93
📒 Files selected for processing (7)
shortcuts/doc/doc_errors_test.goshortcuts/doc/docs_update_test.goshortcuts/doc/docs_update_v2.goskills/lark-doc/references/lark-doc-update.mdtests/cli_e2e/docs/coverage.mdtests/cli_e2e/docs/docs_update_dryrun_test.gotests/cli_e2e/docs/docs_update_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/doc/docs_update_test.go
- skills/lark-doc/references/lark-doc-update.md
- shortcuts/doc/docs_update_v2.go
| deleteTargets := []string{ | ||
| "range-delete-a-" + suffix, | ||
| "range-delete-b-" + suffix, | ||
| "range-delete-c-" + suffix, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a block after the deletion range.
The deletion range ends at the final fixture block. A delete operation that removes one or more blocks after --end-block-id will pass this test.
Add a distinct trailing block. Assert that it remains after deletion.
Proposed test change
deleteTargets := []string{
"range-delete-a-" + suffix,
"range-delete-b-" + suffix,
"range-delete-c-" + suffix,
}
+ deleteTail := "range-delete-tail-" + suffix
replacement := "range-replaced-" + suffix
folderToken := drive.CreateDriveFolder(t, parentT, ctx, "lark-cli-e2e-range-folder-"+suffix, defaultAs, "")
docToken := createDocWithRetry(t, parentT, ctx, folderToken, "lark-cli-e2e-range-"+suffix,
- strings.Join(append(append([]string{}, replaceTargets...), deleteTargets...), "\n\n"), defaultAs)
+ strings.Join(append(append(append([]string{}, replaceTargets...), deleteTargets...), deleteTail), "\n\n"), defaultAs)
// ...
- return strings.Contains(afterDelete, replacement) && !containsAny(afterDelete, deleteTargets), nil
+ return strings.Contains(afterDelete, replacement) &&
+ strings.Contains(afterDelete, deleteTail) &&
+ !containsAny(afterDelete, deleteTargets), nilAs per coding guidelines, “Every behavior change requires a nearby regression test that fails when the implementation is reverted; tests should assert fields, requests, typed errors, or side effects directly.”
Also applies to: 161-169
🤖 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 `@tests/cli_e2e/docs/docs_update_test.go` around lines 97 - 101, Add a distinct
trailing fixture block after the final entry in the deletion range used by the
test, then assert after the delete operation that this trailing block still
exists. Update the nearby assertions in the test’s deletion flow so the
regression specifically verifies blocks after end-block-id are preserved.
Source: Coding guidelines
|
|
3 participants
Summary
Expose inclusive range selectors in docs +update for both block_replace and block_delete. The revision pinning/version behavior found during PPE testing is intentionally excluded and will be handled separately.
Changes
Test Plan
Related Issues
Dependencies
Summary by CodeRabbit
New Features
--start-block-idand--end-block-idoptions for defining operation ranges.Bug Fixes
Documentation