fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema#1030
Open
zhengzhijiej-tech wants to merge 4 commits into
Conversation
…ort with server schema Five separate E2E failures in shortcuts/sheets/ that all trace back to a CLI ↔ server contract mismatch. Each is independently scoped; bundling them because they share the test-report citation and the same one-line fix shape in most cases. larksuite#1 +workbook-create initial fill: omit sheet_id, use sheet_name "Sheet1" buildInitialFillInput sent {"sheet_id": ""} on the secondary set_cell_range call after creating the workbook. The empty value was a holdover from "...otherwise server picks first sheet" — but set_cell_range rejects an empty selector with "sheet_id or sheet_name is required" rather than falling back to the default sheet. Use sheet_name "Sheet1" instead. POST /sheets/v3/spreadsheets always creates that sheet on workbook creation, and set_cell_range accepts sheet_name as an equivalent selector — saves an extra get_workbook_structure round-trip just to learn the auto-generated id. larksuite#2 +dropdown-set / -update data_validation: 4 field renames + 2 deletes buildDropdownValidation emitted four fields that don't exist in the canonical set_cell_range.data_validation schema: - "values" (options list) → renamed to "items" - "multiple_values" → renamed to "support_multiple_values" - "colors" (per-option color) → removed (not in schema; flag also removed from data/flag-defs.json for +dropdown-set / -update) - "highlight_options" → removed (not in schema; flag also removed) The canonical schema lives at sheet-skill-spec/canonical-spec/tool- schemas/mcp-tools.json (set_cell_range tool, data_validation property); the colors / highlight knobs were CLI inventions the server never accepted, so removing the flags is correct (renaming would leave the flags broken). Skill reference docs (write-cells.md, batch-update.md) synced. validateDropdownOptionsColors lost its colors check; renamed to validateDropdownOptions to reflect the narrower contract. larksuite#3 +dropdown-get: split sheet prefix from range, pass via sheet selector dropdownGetInput sent "Sheet1!C2:C6" verbatim as a ranges[] entry. get_cell_ranges expects sheet_id / sheet_name as separate fields and ranges entries without the sheet prefix; the server bounced with "sheet not found, sheetId:" (empty). Use the existing splitSheetPrefixedRange helper (declared in lark_sheet_batch_update.go) to break "Sheet1!C2:C6" into ("Sheet1", "C2:C6"), then thread the sheet name through sheetSelectorForToolInput exactly like +cells-get does. larksuite#4 +dim-move: switch endpoint to /move_dimension + body key snake_case The shortcut was POSTing to /sheets/v2/spreadsheets/{token}/dimension_ range, which is the v2 insert-dimension endpoint and requires a top- level {"dimension": {...}} body. Move uses a separate endpoint: POST /sheets/v2/spreadsheets/{token}/move_dimension body: { "source": {...}, "destination_index": N } (camelCase "destinationIndex" → snake_case "destination_index" to match the v2 contract.) Both DryRun and Execute updated, plus the TestDimMove_DryRun and TestExecute_DimMove assertions. larksuite#5 +range-sort: pre-check sort_conditions[i].column + .ascending transform_range.sort_conditions[i] requires both `column` (string) and `ascending` (bool); rangeSortInput passed the --sort-keys array through to the server unvalidated, so missing fields surfaced as opaque "required property X missing" errors with no per-item context. Walk the parsed array client-side, reject with item-pointing messages. Test fixtures and a contract-test fixture switched from the historical {col, order} vocabulary (which the server has never accepted) to the correct {column, ascending}. Server-schema citations and test-report case mapping in this branch's plan file.
|
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
data/flag-defs.json is regenerated from the upstream sheet-skill-spec
canonical-spec; editing it here gets clobbered on the next sync. The
schema realignment for +dropdown-set / -update --colors / --highlight
removal needs to land on the base table first, then flow back through
sheet-skill-spec → larksuite-cli sync, not via a direct CLI-side edit.
Restore the previous flag entries verbatim. The Go-side change in
buildDropdownValidation still drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until the upstream removal lands;
- after upstream removal + sync, both the flag declarations and the
Go-side handling will be in sync.
Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
These md files are sync targets generated from sheet-skill-spec; editing
them here gets clobbered on the next sync, same as data/flag-defs.json.
The --colors / --highlight row removals belong on the upstream base
table → canonical-spec sync, not here.
Restore the previous --colors / --highlight rows in both
lark-sheets-write-cells.md (+dropdown-set) and lark-sheets-batch-update.md
(+dropdown-update). The Go-side change in buildDropdownValidation still
drops the wire fields, so:
- users passing --colors / --highlight today see the flag accepted
silently (no effect on the wire) until upstream removes the flag;
- after upstream removal + sync, both flag declarations, ref docs, and
Go-side handling will be in sync.
Functional fixes (larksuite#1 workbook-create, larksuite#3 dropdown-get, larksuite#4 dim-move,
larksuite#5 range-sort) and dropdown wire-shape rename (larksuite#2) are unaffected.
github-actions
Bot
added
size/M
… --highlight Upstream sheet-skill-spec base table deleted the --colors and --highlight flags on +dropdown-set / +dropdown-update (the corresponding wire fields data_validation.colors / .highlight_options were never accepted by the server schema; see prior fix in this branch). Re-running the sync from canonical-spec brings the CLI flag-defs and skill reference docs back in line with the Go-side handling that already drops these fields. Generated by `npm run sync:cli` in sheet-skill-spec @ ac7acef.
github-actions
Bot
added
size/L
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
Five independent schema-alignment fixes surfaced by the E2E test report on
this branch (commit `1e05e7b`, PPE lane `ppe_lark_cli_sheet`). Each
matches a specific server error that traces back to a CLI ↔
canonical-schema mismatch.
Fixes per item
#1 workbook-create initial fill — replace `{"sheet_id":""}` with
`{"sheet_name":"Sheet1"}`. New workbooks always start with that
default sheet; using sheet_name avoids an extra `get_workbook_structure`
round-trip just to learn the auto-generated id.
`lark_sheet_workbook.go:694-699`
#2 dropdown data_validation — align with canonical schema in
`sheet-skill-spec/canonical-spec/tool-schemas/mcp-tools.json`:
`--colors` / `--highlight` flags (server never accepted them; the
flags were CLI inventions)
Skill ref docs (`lark-sheets-write-cells.md`, `lark-sheets-batch-update.md`)
synced.
`lark_sheet_write_cells.go:330-358`
#3 dropdown-get range prefix — reuse the existing
`splitSheetPrefixedRange` helper to break `Sheet1!C2:C6` into
sheet_name + bare A1, then thread through
`sheetSelectorForToolInput` (same pattern as `+cells-get`).
`lark_sheet_read_data.go:249-262`
#4 dim-move endpoint + body key — switch URL from
`/dimension_range` (which is v2 insert) to `/move_dimension`; rename
body key `destinationIndex` → `destination_index` for v2 contract
consistency.
`lark_sheet_sheet_structure.go:573,598,621`
#5 range-sort item validation — walk `--sort-keys` client-side,
reject items missing `column` (string) or `ascending` (bool) with
per-item error messages. Test fixtures and the batch-op contract fixture
switched from the historical `{col, order}` vocabulary (which the
server has never accepted) to the correct `{column, ascending}`.
`lark_sheet_range_operations.go:590-625`
Test plan
030 / TC-046 / TC-038 / R2-003006 / R2-009 / R2-012