feat(drive): switch markdown export to V2 docs_ai fetch API#948
Conversation
|
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:
📝 WalkthroughWalkthroughSwitches drive markdown export to support ChangesDrive Export Markdown Endpoint Migration
Sequence DiagramsequenceDiagram
participant DryRun
participant Execute
participant DocsAI as DocsAI Endpoint
participant File as Output File
DryRun->>DocsAI: POST /open-apis/docs_ai/v1/documents/{encoded_token}/fetch {"format":"markdown"}
DocsAI-->>DryRun: Dry-run plan/URL
Execute->>DocsAI: POST /open-apis/docs_ai/v1/documents/{encoded_token}/fetch {"format":"markdown"}
DocsAI-->>Execute: {data: {document: {content: "..."}}}
Execute->>File: Write []byte(content)
Execute->>File: Record size_bytes = len(content)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
5bb77c0 to
fec448c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/drive/drive_export_test.go (1)
84-95: ⚡ Quick winAssert the markdown fetch request body includes
{"format":"markdown"}.These tests currently verify URL/method but not the POST payload contract. Capturing and asserting
CapturedBodywould catch regressions in the new docs_ai request shape.Suggested pattern (apply to markdown fetch tests)
- reg.Register(&httpmock.Stub{ + fetchStub := &httpmock.Stub{ Method: "POST", URL: "/open-apis/docs_ai/v1/documents/docx123/fetch", Body: map[string]interface{}{ "code": 0, "data": map[string]interface{}{ "document": map[string]interface{}{ "content": "# hello\n", }, }, }, - }) + } + reg.Register(fetchStub) @@ if err != nil { t.Fatalf("unexpected error: %v", err) } + var req map[string]interface{} + if err := json.Unmarshal(fetchStub.CapturedBody, &req); err != nil { + t.Fatalf("unmarshal docs_ai fetch body: %v", err) + } + if req["format"] != "markdown" { + t.Fatalf("docs_ai fetch body format = %v, want %q", req["format"], "markdown") + }Also applies to: 137-148, 240-251
🤖 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_export_test.go` around lines 84 - 95, Update the httpmock registration in drive_export_test.go so the POST stub for "/open-apis/docs_ai/v1/documents/docx123/fetch" captures and asserts the request body contains {"format":"markdown"}: modify the httpmock.Stub registered via reg.Register to include a CapturedBody (or use Stub’s capture mechanism) and add an assertion in the test that the captured body has format == "markdown"; apply the same change to the other markdown fetch tests referenced (the stubs around the blocks at the other two locations mentioned) so all markdown fetch tests validate the POST payload shape.
🤖 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/drive/drive_export.go`:
- Around line 122-126: The parsing currently silently allows content to be empty
by doing content, _ = doc["content"].(string); change this to fail fast when
document or document.content is missing or not a string: check that
data["document"] is a map[string]interface{} and that doc["content"] exists and
is a non-empty string, and if not return/propagate a clear error (or log and
abort export) instead of proceeding to write an empty file; update the parsing
block that references data, document, and content to validate types and surface
an error upstream so callers of the export routine know the response shape was
invalid.
---
Nitpick comments:
In `@shortcuts/drive/drive_export_test.go`:
- Around line 84-95: Update the httpmock registration in drive_export_test.go so
the POST stub for "/open-apis/docs_ai/v1/documents/docx123/fetch" captures and
asserts the request body contains {"format":"markdown"}: modify the
httpmock.Stub registered via reg.Register to include a CapturedBody (or use
Stub’s capture mechanism) and add an assertion in the test that the captured
body has format == "markdown"; apply the same change to the other markdown fetch
tests referenced (the stubs around the blocks at the other two locations
mentioned) so all markdown fetch tests validate the POST payload shape.
🪄 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: e4a4a6ea-1fd1-4438-a402-138f27f0d7f8
📒 Files selected for processing (11)
shortcuts/common/drive_meta.goshortcuts/common/resource_url.goshortcuts/common/resource_url_test.goshortcuts/drive/drive_export.goshortcuts/drive/drive_export_common.goshortcuts/drive/drive_export_test.goshortcuts/drive/drive_info.goshortcuts/drive/shortcuts.goshortcuts/drive/shortcuts_test.goshortcuts/sheets/lark_sheets_cell_style_and_merge.gotests/cli_e2e/drive/drive_info_dryrun_test.go
💤 Files with no reviewable changes (1)
- shortcuts/drive/drive_export_common.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0aa7f3245d4be249aab6e1b5267b9f12b9ec6325🧩 Skill updatenpx skills add larksuite/cli#feat/drive-export-markdown-v2 -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #948 +/- ##
==========================================
+ Coverage 66.71% 67.39% +0.68%
==========================================
Files 563 572 +9
Lines 52287 53659 +1372
==========================================
+ Hits 34884 36165 +1281
+ Misses 14509 14486 -23
- Partials 2894 3008 +114 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fec448c to
efbd29c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/drive/drive_export.go (1)
122-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when
document.contentis missing or invalid.Line 122–126 silently accepts malformed responses and can write an empty file while reporting success. Return an API error if
documentordocument.contentis absent/not a string.Suggested fix
- // Extract content from the V2 response: data.document.content - var content string - if doc, ok := data["document"].(map[string]interface{}); ok { - content, _ = doc["content"].(string) - } + // Extract content from the V2 response: data.document.content + doc, ok := data["document"].(map[string]interface{}) + if !ok { + return output.Errorf(output.ExitAPI, "api_error", "invalid markdown fetch response: missing document object") + } + content, ok := doc["content"].(string) + if !ok { + return output.Errorf(output.ExitAPI, "api_error", "invalid markdown fetch response: missing document.content") + }🤖 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_export.go` around lines 122 - 126, The code currently quietly accepts a missing or non-string document.content (the block that sets var content from data["document"]) which can result in writing an empty file; update the handler in drive_export.go to validate presence and type of data["document"] and doc["content"] after the cast, and if either is missing or not a string return a clear API error (e.g., HTTP 500/400 via the existing error-return path) instead of proceeding—refer to the variables data, doc and content in the current extraction block and use the handler's existing error response mechanism to fail fast.
🤖 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.
Duplicate comments:
In `@shortcuts/drive/drive_export.go`:
- Around line 122-126: The code currently quietly accepts a missing or
non-string document.content (the block that sets var content from
data["document"]) which can result in writing an empty file; update the handler
in drive_export.go to validate presence and type of data["document"] and
doc["content"] after the cast, and if either is missing or not a string return a
clear API error (e.g., HTTP 500/400 via the existing error-return path) instead
of proceeding—refer to the variables data, doc and content in the current
extraction block and use the handler's existing error response mechanism to fail
fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fc28197-5276-4498-ad81-640c08b153a7
📒 Files selected for processing (2)
shortcuts/drive/drive_export.goshortcuts/drive/drive_export_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/drive/drive_export_test.go
fd561a6 to
31cee95
Compare
Switch `drive +export --file-extension markdown` from the legacy V1
GET /open-apis/docs/v1/content API to the V2
POST /open-apis/docs_ai/v1/documents/{token}/fetch API for
higher-quality Lark-flavored Markdown output.
- Update DryRun and Execute paths to use V2 endpoint with JSON body
- Add docx:document:readonly scope for the new API
- Validate V2 response structure (fail fast on missing document/content)
- Encode token in URL path via validate.EncodePathSegment
- Update unit tests and add V2 response validation error path tests
- Add E2E dry-run test for markdown export path
- Update skill documentation
04163cb to
0aa7f32
Compare
…e#948) Switch `drive +export --file-extension markdown` from the legacy V1 GET /open-apis/docs/v1/content API to the V2 POST /open-apis/docs_ai/v1/documents/{token}/fetch API for higher-quality Lark-flavored Markdown output. - Update DryRun and Execute paths to use V2 endpoint with JSON body - Add docx:document:readonly scope for the new API - Validate V2 response structure (fail fast on missing document/content) - Encode token in URL path via validate.EncodePathSegment - Update unit tests and add V2 response validation error path tests - Add E2E dry-run test for markdown export path - Update skill documentation
Summary
drive +export --file-extension markdownpreviously used the V1 content API (GET /open-apis/docs/v1/content) which returns HTML-oriented markdown with poor readability:<table>HTML tags (not standard Markdown| | |syntax)\-,\.,\+,\&\#34;)<div class="callout">instead of<callout emoji="💡">)Switch to the V2 docs_ai fetch API (
POST /open-apis/docs_ai/v1/documents/{token}/fetchwithformat=markdown) which produces clean Lark-flavored Markdown with standard table syntax, preserved semantic tags, and compact formatting.The file-writing logic (output dir, filename resolution via
FetchDriveMetaTitle, overwrite policy) is unchanged.Changes
shortcuts/drive/drive_export.go: DryRun and Execute paths switch from V1 content API to V2 docs_ai fetch API; adddocx:document:readonlyscopeshortcuts/drive/drive_export_test.go: Update 4 test cases to mock the new V2 endpointTest Plan
go test -race -count=1 -run "TestDriveExport|TestSaveContent|TestValidateDriveExportSpec" ./shortcuts/drive/...— all passgo vet ./shortcuts/drive/...— cleanlark-cli drive +export --token <token> --doc-type docx --file-extension markdown— verify output is Lark-flavored Markdown (standard tables, callout with emoji, no excessive escaping)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores