docs: add topic move collector workflow#1473
Open
YH-1600 wants to merge 3 commits into
Open
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:
📝 WalkthroughWalkthroughAdds a complete ChangesTopic-Move-Collector Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
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 #1473 +/- ##
=======================================
Coverage 73.42% 73.42%
=======================================
Files 753 753
Lines 70111 70111
=======================================
Hits 51476 51476
Misses 14826 14826
Partials 3809 3809 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9d7db890e7f67695e3a3a2e47b525369211ad9c6🧩 Skill updatenpx skills add larksuite/cli#feat/workflow-topic-move-collector -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@skills/lark-drive/references/lark-drive-workflow-topic-move-collector-execute.md`:
- Line 64: The logic at line 64 references a `create_required=true` condition to
determine whether to create targets before moving, but the `MovePlanItem` schema
definition at lines 149–164 does not include a `create_required` field. Resolve
this inconsistency by either: (1) adding a `create_required` boolean field to
the `MovePlanItem` schema, or (2) rewriting the condition at line 64 to check
`action_type == "create_target"` instead of using the missing `create_required`
field. Ensure the chosen approach is consistently applied in both locations so
the schema and the documented logic match.
- Line 67: The timing description for `rollback_snapshot` recording is
incorrect. Change the statement from "每次移动前记录 `rollback_snapshot`" (record
snapshot before each move) to specify that the snapshot should be built once
before the first write operation, not before each individual move. This aligns
with the established write-operation snapshot contract where the snapshot
captures original locations for the entire confirmed scope upfront, enabling
recovery if any write fails partway through, consistent with how it is
documented in reference workflows like
lark-drive-workflow-knowledge-organize-execution.md.
- Around line 177-192: The ExecutionJournal schema in the execution log section
is missing critical fields required by the recovery workflow. Expand the JSON
schema to include plan_id (to match journal entries to original plan items),
returned_token and returned_parent_token (to identify current resource locations
for recovery operations), task_id (to track async operation completion),
created_by_workflow (to distinguish resources created by this workflow run for
cleanup decisions), and rollback_eligible (to filter reversible operations).
Additionally, restructure the schema by adding journal_id as a stable
identifier, replacing source and target with input_token and returned_token
pairs, adding target_parent_token and returned_parent_token, introducing an
operation field for granular tracking, and changing status to include the
pending state. This alignment ensures the recovery workflow can generate
accurate recovery plans and handle cleanup decisions based on the execution
journal alone.
In `@skills/lark-drive/references/lark-drive-workflow-topic-move-collector.md`:
- Line 120: The CONFIRM_EXECUTION state in the state machine table is missing a
transition back to PLAN_MOVE. According to the UI specification, users can
choose to "adjust plan" (调整计划) from the write operation confirmation UI, which
should return them to the PLAN_MOVE state. Update the "next states" column in
the CONFIRM_EXECUTION row to include PLAN_MOVE in addition to the currently
listed EXECUTE or DONE transitions, reflecting all possible paths a user can
take from that state.
🪄 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: addb433b-0a41-42e0-b0d1-06eba255a5dd
📒 Files selected for processing (7)
skills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-execute.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-recall.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-review-plan.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector.mdskills/lark-wiki/SKILL.md
YH-1600
force-pushed
the
feat/workflow-topic-move-collector
branch
3 times, most recently
from
899c666 to
403ac2d
Compare
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
`@skills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.md`:
- Around line 109-122: Update the `resolve_status` enum in the TargetLocation
schema to properly cover all documented RESOLVE_TARGET outcomes. Add `unknown`
to the enum values since it represents a valid exit state when ambiguity
requires user selection. For the `permission_denied` value, either add
documentation explaining the triggering conditions that produce this outcome, or
remove it if it is not a valid RESOLVE_TARGET result. Additionally, review the
`resolve_status` field definition in review-plan.md to ensure schema consistency
across workflow phases, as it currently shows a different enum there with values
`resolved|partial|failed` which conflicts with the setup.md definition.
In `@skills/lark-drive/references/lark-drive-workflow-topic-move-collector.md`:
- Around line 126-133: The progressive loading table references workflow states
that have not yet been implemented in their corresponding documentation files.
Implement the 5 missing state sections: add the CONFIRM_CONTEXT section to
lark-drive-workflow-topic-move-collector-setup.md (which currently documents
PARSE_INPUT and RESOLVE_TARGET); add the RELEVANCE_CLASSIFY and PLAN_MOVE
sections to lark-drive-workflow-topic-move-collector-review-plan.md (which
currently documents RESOURCE_RESOLVE and CONTENT_VERIFY); and add the VERIFY and
RESTORE sections to lark-drive-workflow-topic-move-collector-execute.md (which
currently documents CONFIRM_EXECUTION and EXECUTE). Note that
lark-drive-workflow-topic-move-collector-recall.md correctly implements both
SEARCH_RECALL and RECALL_ENHANCE states it references. Complete these
implementations before merging this change.
🪄 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: 5a1153b5-f922-4fdd-817b-f479b599f10d
📒 Files selected for processing (7)
skills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-execute.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-recall.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-review-plan.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector.mdskills/lark-wiki/SKILL.md
✅ Files skipped from review due to trivial changes (3)
- skills/lark-drive/references/lark-drive-workflow-topic-move-collector-review-plan.md
- skills/lark-drive/references/lark-drive-workflow-topic-move-collector-execute.md
- skills/lark-drive/references/lark-drive-workflow-topic-move-collector-recall.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/lark-wiki/SKILL.md
YH-1600
force-pushed
the
feat/workflow-topic-move-collector
branch
from
403ac2d to
dbfa19d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.md (1)
109-134:⚠️ Potential issue | 🟠 Major
permission_deniedinresolve_statusenum is undocumented in the RESOLVE_TARGET state definition.Line 120 includes
permission_deniedin theresolve_statusenum forTargetLocation, but the RESOLVE_TARGET state definition (lines 51–77) does not document any condition that produces this status. The must-dos cover token parsing and marking unsupported move directions, but do not specify when permission is checked or whenpermission_deniedwould be returned.Either document the condition that triggers
permission_denied, or remove it from the enum if it is not a valid outcome. Note that in review-plan.md, permission concerns appear to be handled through a separatepermission_statefield, suggestingpermission_deniedmay belong there rather than inresolve_status.Additionally,
TargetLocation.resolve_status(setup.md) andResourceItem.resolve_status(review-plan.md) use the same field name with different enum values (resolved|ambiguous|unsupported|permission_deniedvs.resolved|partial|failed). While these serve different purposes in different lifecycle phases, reusing the same field name with conflicting enums is error-prone. Consider renaming one field for clarity (e.g.,target_resolve_statusvs.item_resolve_status).🤖 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 `@skills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.md` around lines 109 - 134, The `resolve_status` enum in the TargetLocation definition includes `permission_denied` as a possible value, but the RESOLVE_TARGET state definition does not document any condition that would produce this status or when permission checking occurs. Either add documentation to the RESOLVE_TARGET state explaining the condition that triggers `permission_denied` (describing when and how permissions are checked), or remove `permission_denied` from the TargetLocation resolve_status enum if it is not a valid outcome for this lifecycle phase. Additionally, the same field name `resolve_status` is reused in both TargetLocation and ResourceItem with different enum values (resolved|ambiguous|unsupported|permission_denied versus resolved|partial|failed), which creates confusion and error-prone code. To address this, rename one of these fields for clarity—consider using `target_resolve_status` for TargetLocation and `item_resolve_status` for ResourceItem—to make their distinct purposes and value sets explicit.
🤖 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.
Outside diff comments:
In
`@skills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.md`:
- Around line 109-134: The `resolve_status` enum in the TargetLocation
definition includes `permission_denied` as a possible value, but the
RESOLVE_TARGET state definition does not document any condition that would
produce this status or when permission checking occurs. Either add documentation
to the RESOLVE_TARGET state explaining the condition that triggers
`permission_denied` (describing when and how permissions are checked), or remove
`permission_denied` from the TargetLocation resolve_status enum if it is not a
valid outcome for this lifecycle phase. Additionally, the same field name
`resolve_status` is reused in both TargetLocation and ResourceItem with
different enum values (resolved|ambiguous|unsupported|permission_denied versus
resolved|partial|failed), which creates confusion and error-prone code. To
address this, rename one of these fields for clarity—consider using
`target_resolve_status` for TargetLocation and `item_resolve_status` for
ResourceItem—to make their distinct purposes and value sets explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2b6ea91-2a57-4c80-b246-2e54eb1dd807
📒 Files selected for processing (7)
skills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-execute.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-recall.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-review-plan.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector-setup.mdskills/lark-drive/references/lark-drive-workflow-topic-move-collector.mdskills/lark-wiki/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- skills/lark-wiki/SKILL.md
- skills/lark-drive/references/lark-drive-workflow-topic-move-collector-review-plan.md
- skills/lark-drive/references/lark-drive-workflow-topic-move-collector-recall.md
YH-1600
force-pushed
the
feat/workflow-topic-move-collector
branch
4 times, most recently
from
ae8ec8a to
32524a8
Compare
Change-Id: Ifc6c8d0b90faabff1136d92db1bc7e5556e77d85
Change-Id: I13992bfa82acaeafb4e5fce460ae47ec8c6ad3af
YH-1600
force-pushed
the
feat/workflow-topic-move-collector
branch
from
32524a8 to
5dde10f
Compare
Change-Id: Id0b8988d3f0385af36bd721549299928a17fbe76
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.
Change-Id: Ifc6c8d0b90faabff1136d92db1bc7e5556e77d85
Summary
Add a new workspace-topic-move-collector workflow for finding Workspace resources by topic or content clues, reviewing relevance, and moving confirmed resources into a Drive folder or Wiki node.
Changes
verification, and restore.
Test Plan
lark-cli <domain> <command>flow works as expectedRelated Issues
Summary by CodeRabbit
drive +search.