feat(sheets): add +history-list / +history-revert / +history-revert-status shortcuts#1653
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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
wenzhuozhen
force-pushed
the
feat/sheet-history-revert
branch
from
June 29, 2026 12:12
d789653 to
b552e69
Compare
…tatus shortcuts BE-1 + BE-2 (larksuite/cli lark-sheets) for spec sheet-history-revert. Three thin callTool wrappers over facade-agg history tools, following the existing sheets Validate/DryRun/Execute + --url/--spreadsheet-token(/--token) locator convention: - +history-list (read, history_list): passes the tool output through verbatim; facade-agg already does the minor_histories/4-field/RFC3339 transform. - +history-revert (write, history_revert): --history-version-id required, enforced at Validate stage with a typed *errs.ValidationError (no request on missing); returns the async receipt. - +history-revert-status (read, history_revert_status): polls in-progress / success / failure. Flags declared inline (not via *_gen.go) — flag_defs_gen.go / data/flag-defs.json are synced from sheet-skill-spec (BE-3) and must not be hand-edited. Notes: - history_revert / history_revert_status depend on facade-agg's downstream RPC wiring, a DEFERRED follow-up; the tools return a "not wired yet" guard today. These CLI wrappers are correct and go live when the backend follow-up lands. +history-list is fully functional now. - TestFlagDefsGen_MatchesJSON fails on baseline (pre-existing BE-3 gen/json drift); resolves once BE-3 sync:cli regenerates flag defs for these shortcuts. Validation: go build ./shortcuts/sheets/... PASS; new tests (TestHistoryShortcuts_DryRun, TestHistoryRevert_MissingVersionID) PASS. Spec source: active@2acd94a24ac3f835357a274a02344f78435bcc1c39ad0d695ce587f0cbddfb21
…kill-spec (BE-3) Synced artifacts for the history shortcuts from ee/sheet-skill-spec (SSOT), landed surgically (history-only) to avoid regressing this branch's newer skills/lark-sheets content: - skills/lark-sheets/references/lark-sheets-history.md (new, mirrored). - skills/lark-sheets/SKILL.md: + Lark Sheet History references-table row only. - shortcuts/sheets/data/flag-defs.json: + 3 history shortcuts (additive; no existing entries touched). - shortcuts/sheets/flag_defs_gen.go: regenerated via go generate ./shortcuts/sheets/... (this also resolves the pre-existing flag-defs/gen drift — TestFlagDefsGen_MatchesJSON now passes). NOT a full mirror: the rest of skills/lark-sheets/ + flag-schemas.json on this branch (feat/lark-sheets-develop) are NEWER than the sheet-skill-spec worktree's canonical (e.g. /wiki/ URL support, schema_version 3). A wholesale sync:cli would have reverted them, so only the history delta is taken here. Full re-sync should happen once sheet-skill-spec canonical is realigned with this branch. Validation: go generate clean; go test ./shortcuts/sheets/ (TestFlagDefsGen_MatchesJSON, TestHistory*) PASS. Spec source: active@2acd94a24ac3f835357a274a02344f78435bcc1c39ad0d695ce587f0cbddfb21
…sion id BE-2 gap surfaced by PPE E2E: +history-revert-status sent history_version_id, but the facade-agg history_revert_status tool keys on transaction_id (the async receipt returned by +history-revert), so it returned "[40400] transaction_id is required". Give the status shortcut its own --transaction-id flag + input (excel_id + transaction_id); revert keeps --history-version-id. Tests updated.
…tFlagsFor) TestFlagsFor_EveryRegisteredCommandHasDefs was RED: generated flag-defs drifted from the hand-written history shortcuts. - +history-revert-status: flag-defs had --history-version-id; the BE-2 fix switched the shortcut to --transaction-id. Updated the entry to transaction-id. - +history-revert / -status --history-version-id were marked required="required", but the inline flags are cobra-optional (requiredness enforced in Validate). Set required="optional" to match. Regenerated flag_defs_gen.go. NOTE: canonical source is sheet-skill-spec (BE-3); apply the same change upstream or the next sync:cli will regress this.
…nsaction-id) Mirror the upstream BE-2 fix in canonical-spec/references/lark_sheet_history/ cli-reference.md: +history-revert-status now uses --transaction-id (taken from the async receipt returned by +history-revert), and +history-revert's --history-version-id flips required→optional (Validate enforces requiredness at runtime). This file is the only history-only delta from the upstream sheet-skill-spec sync; the rest of skills/lark-sheets/ stays on the cli's newer baseline (/wiki/ URL support, +cells-set-image / +float-image-create, etc.) to match commit 8ae516d's history-only mirror policy. Spec source companion change: feat/sheet-history-revert in ee/sheet-skill-spec, canonical-spec/{tool-shortcut-map.json,references/ lark_sheet_history/cli-reference.md}.
Spec follow-up sheet-history-revert: thread the history_list pagination
contract through the +history-list shortcut.
- shortcuts/sheets/lark_sheet_history_list.go:
+ --end-version (int, optional). Mapped to the tool input's `end_version`
only when explicitly set (so the server treats absence as
"first page / latest"), via runtime.Changed / runtime.Int (matches the
+formula-verify --max-locations precedent).
+ Tip: pass next_end_version from the response on the next call;
capture exits the pagination loop when the server omits the field.
- shortcuts/sheets/lark_sheet_history_test.go: + dry-run case asserting
--end-version 12345 lands as input.end_version=12345 (post-JSON
unmarshal float64).
- skills/lark-sheets/references/lark-sheets-history.md: synced from
ee/sheet-skill-spec (commit 39c6b61). Adds the "倒序分页" caveat row +
--end-version flag + pagination Examples line. Drops the internal
MajorHistory.Version implementation detail per spec follow-up.
- shortcuts/sheets/data/flag-defs.json: synced from spec (+history-list
+--end-version int optional).
- shortcuts/sheets/flag_defs_gen.go: regenerated via
`go generate ./shortcuts/sheets/...`.
Companion changes:
- ee/sheet-skill-spec MR !37: spec-tables + tool-schemas pagination
contract (commits 09e8604, 39c6b61).
- ee/sheet-facade-agg MR !1028: history_list tool plumbs end_version,
emits next_end_version + has_more (omitted at earliest page),
defaults PageSize=20 to datarpc.
Validation:
- go build ./shortcuts/sheets/... PASS
- go test ./shortcuts/sheets/... PASS (sheets + backward)
- TestHistoryShortcuts_DryRun (5 cases incl. new --end-version case): PASS
- TestHistoryRevert_MissingRequiredFlag: PASS
- TestFlagsFor_EveryRegisteredCommandHasDefs: PASS
- TestFlagDefsGen_MatchesJSON: PASS
… + revert max-cells default drift Two issues surfaced during MR !37 review: 1) +history-revert --history-version-id requiredness was set as "optional" in the spec table (BE-2 fix dc5fe0e) so cobra wouldn't block before Validate. Per upstream review the flag should be required-by-cobra so the user gets the standard "required flag(s)" gate immediately and the runtime contract matches the JSON shape. - shortcuts/sheets/lark_sheet_history_revert.go: historyVersionIDFlag now sets Required: true. Validate keeps a trim/empty-string guard so '--history-version-id ""' still fails as a typed *errs.ValidationError (cobra accepts empty strings as "set"). - shortcuts/sheets/data/flag-defs.json: +history-revert --history-version-id required: optional -> required. - shortcuts/sheets/flag_defs_gen.go: regenerated. - shortcuts/sheets/lark_sheet_history_test.go: TestHistoryRevert_MissingRequiredFlag split into per-shortcut subtests; +history-revert asserts cobra's "required flag(s)" contract (raw err — the test rig calls cmd.Execute directly so it doesn't see the cmd dispatcher's typed envelope wrap); +history-revert-status keeps the typed *errs.ValidationError contract (its --transaction-id stays cobra-optional + Validate-enforced). 2) max-cells safety cap was accidentally rewritten from 200000 to 50000 by the last sync from sheet-skill-spec (the spec canonical side fell out of date — fixed separately on the spec MR follow-up). Restore desc: "Safety cap; default 200000" / default: "200000" so +cells-get / +csv-get keep the documented cap. Validation: - go test ./shortcuts/sheets/... PASS - TestHistoryRevert_MissingRequiredFlag (both subtests) PASS - TestHistoryShortcuts_DryRun (incl. +history-list pagination case) PASS - TestFlagsFor_EveryRegisteredCommandHasDefs PASS - TestFlagDefsGen_MatchesJSON PASS
…red (match +history-revert) Companion to commit 6ca35b0: same gating model now applies to both history receipts. - shortcuts/sheets/lark_sheet_history_revert.go: transactionIDFlag.Required=true. Validate keeps a trim/empty-string guard for '--transaction-id ""'. - shortcuts/sheets/data/flag-defs.json: +history-revert-status --transaction-id required: optional -> required (synced from sheet-skill-spec @9ca814d). - shortcuts/sheets/flag_defs_gen.go: regenerated. - shortcuts/sheets/lark_sheet_history_test.go: TestHistoryRevert_MissingRequiredFlag/+history-revert-status moved to the cobra "required flag(s)" text contract (the test rig invokes the shortcut via cmd.Execute, which sees the raw cobra error directly without the dispatcher's typed wrap). Drop now-unused `errors` and `errs` imports. Validation: - go test ./shortcuts/sheets/... PASS (sheets + backward) - TestFlagsFor_EveryRegisteredCommandHasDefs: PASS - TestFlagDefsGen_MatchesJSON: PASS - TestHistoryRevert_MissingRequiredFlag (both subtests): PASS
Companion to commit 9fa7331 (transaction-id) and 6ca35b0 (history-version-id): the two flag tables in skills/lark-sheets/references/lark-sheets-history.md still showed 'optional' even though the canonical contract — and shortcuts/sheets/data/ flag-defs.json — already moved to 'required'. The earlier syncs only picked up the data file from spec; the skill markdown drift slipped through. Pull in the spec-side regenerated reference (ee/sheet-skill-spec @9ca814d) so the human-readable doc matches the wire contract.
wenzhuozhen
force-pushed
the
feat/sheet-history-revert
branch
from
June 30, 2026 06:48
b094539 to
017d752
Compare
xiongyuanwen-byted
approved these changes
Jun 30, 2026
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
3 participants
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
Add the Sheet history-revert capability — 3 thin shortcuts wrapping the upstream
history_list/history_revert/history_revert_statustools inee/sheet-facade-agg. Lets users list a spreadsheet's edit history, roll back to a chosen version, and poll the async revert status.End-to-end PPE verification green; spec / facade-agg / sheet-data already merged or in MR.
Changes
+history-list(read, history_list): listminor_historiesfor a spreadsheet (workbook-level; works on--url/--spreadsheet-token); facade-agg already returns the 4-field RFC3339 transform.+history-revert(write, history_revert): submit an async revert to a chosen--history-version-id; returns the async receipt (transaction_id).+history-revert-status(read, history_revert_status): poll the async revert by--transaction-id(the receipt from+history-revert, NOT the history_version_id).skills/lark-sheets/references/lark-sheets-history.md: synced from sheet-skill-spec (BE-3); SKILL.md gets the new History row.shortcuts/sheets/data/{flag-defs,flag-schemas}.json: synced from spec-tables/*.json SoT.shortcuts/sheets/flag_defs_gen.go: regenerated viago generate ./shortcuts/sheets/....Companion changes:
Test Plan
go build ./shortcuts/sheets/...cleango test -run 'TestHistory|TestFlagsFor|TestFlagDefsGen' ./shortcuts/sheets/...passRelated Issues