fix(sheets): align +workbook-create, +dropdown-*, +dim-move, +range-sort with server schema (restore --colors/--highlight)#1030
Open
zhengzhijiej-tech wants to merge 5 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
…al fields Reverses the --colors / --highlight removal from 7932ab2 (item larksuite#2 of the batch-1 schema-alignment commit). That commit dropped both flags after the test report flagged data_validation.colors / highlight_options as "unexpected property" — at the time the canonical set_cell_range.data_validation schema listed only help_text / items / operator / range / support_multiple_values / type / values, so the flags had no server-side target and the removal was correct. Since then, set_cell_range.data_validation has gained two fields explicitly modelling the dropdown highlight UI (mcp-tools.json in sheet-skill-spec 2026-05-22 base sync): enable_highlight (bool) — show pill backgrounds highlight_colors (string[]) — hex pill colors, length must match items So the flags are back, but rewired: --colors -> data_validation.highlight_colors (was: colors) --highlight -> data_validation.enable_highlight (was: highlight_options) --options -> items and --multiple -> support_multiple_values renames from 7932ab2 are kept. Changes: - buildDropdownValidation: re-add --colors / --highlight handling against the new field names; --colors length check stays inline (so dropdownSetInput Validate path catches it via validateViaInput, no separate guard needed). - validateDropdownOptions -> validateDropdownOptionsColors: restore the Validate-time --colors length check on +dropdown-update / +dropdown-delete (called from lark_sheet_batch_update.go). - TestDropdownSet_CellsShape: extend to assert highlight_colors / enable_highlight emitted; assert legacy `colors` / `highlight_options` absent. - TestDropdownSet_ColorsLengthMismatch: new — covers the early Validate error path. - TestDropdownUpdate_BatchPayload: extend to cover dropdownBatchInput propagation of --colors / --highlight through batch_update. - skills/lark-sheets/references/lark-sheets-{write-cells,batch-update}.md, shortcuts/sheets/data/flag-defs.json, flag-schemas.json: synced from sheet-skill-spec generate output (MR !7).
zhengzhijiej-tech
changed the title
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
Six independent schema-alignment fixes for E2E test report cases on this branch
(commit
1e05e7b, PPE laneppe_lark_cli_sheet). Each matches a specificserver error that traces back to a CLI ↔ canonical-schema mismatch.
+workbook-create --headers/--valuesset_cell_range: sheet_id or sheet_name is requiredbuildInitialFillInputsent{"sheet_id":""}(assumed server-side default fallback that doesn't exist)+dropdown-set / -updatedata_validation.{colors,multiple_values,highlight_options} unexpected propertyset_cell_range.data_validation+dropdown-getsheet not found, sheetId:(empty)dropdownGetInputsentSheet1!C2:C6verbatim instead of splitting into sheet_name + bare A1+dim-move[9499] Missing required parameter: Dimension/dimension_range= insert) + camelCase body key+range-sortrequired property "column"/"ascending" missing--sort-keysitems passed through unvalidated; field-naming drift+dropdown-set / -update(follow-up)--colors/--highlightrevived after canonical schema gainedhighlight_colors/enable_highlightFixes 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_structureround-trip just tolearn the auto-generated id.
lark_sheet_workbook.go:694-699#2 dropdown data_validation field renames — align body fields with
canonical schema in
sheet-skill-spec/canonical-spec/tool-schemas/mcp-tools.json:values→items(option list)multiple_values→support_multiple_values(The
colors/highlight_optionsdeletion that was originally part of thisitem has been undone by #6 below — keep reading.)
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
splitSheetPrefixedRangehelper to break
Sheet1!C2:C6into sheet_name + bare A1, then thread throughsheetSelectorForToolInput(same pattern as+cells-get).lark_sheet_read_data.go:249-262#4 dim-move endpoint + body key — switch URL from
/dimension_range(v2 insert) to
/move_dimension; rename body keydestinationIndex→destination_indexfor v2 contract consistency.lark_sheet_sheet_structure.go:573,598,621#5 range-sort item validation — walk
--sort-keysclient-side, reject itemsmissing
column(string) orascending(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#6 dropdown --colors / --highlight restored, rewired to canonical fields
(commit
514b5e1) — partial revert of #2.When #2 landed, canonical
set_cell_range.data_validationonly knewhelp_text / items / operator / range / support_multiple_values / type / values, so--colors/--highlighthad no server-side target and wereremoved. The canonical schema has since gained two fields explicitly modelling
the dropdown highlight UI (
sheet-skill-specMR!7,
2026-05-22 base sync):
So the flags are back, but rewired:
--colors(string array, hex)highlight_colorscolors--highlight(bool)enable_highlighthighlight_optionsbuildDropdownValidation: re-emit both fields; keep the inlinelen(colors) == len(options)check (the +dropdown-set Validate path catches it viavalidateViaInput).validateDropdownOptions→validateDropdownOptionsColors: restore early Validate-time--colorslength check for +dropdown-update / +dropdown-delete (called fromlark_sheet_batch_update.go).TestDropdownSet_CellsShape,TestDropdownUpdate_BatchPayload, newTestDropdownSet_ColorsLengthMismatch).skills/lark-sheets/references/lark-sheets-{write-cells,batch-update}.md,shortcuts/sheets/data/{flag-defs,flag-schemas}.json.The
--options → itemsand--multiple → support_multiple_valuesrenamesfrom #2 are preserved — only the
--colors/--highlightdeletion isundone.
Test plan
go test ./shortcuts/sheets/... -count=1green (full suite + new dropdown coverage)TestDimMove_DryRun,TestExecute_DimMove,TestDropdownSet_CellsShape,TestDropdownSet_ColorsLengthMismatch,TestDropdownGettable,TestWorkbookCreate_DryRun,TestRangeSort_RejectsMalformedKeys,TestBatchOp_BodyMatchesStandalone/+range-sort,TestDropdownUpdate_BatchPayload)go build ./...greenppe_lark_cli_sheet): replay TC-007 / TC-027030 / TC-046 / TC-038 / R2-003006 / R2-009 / R2-012; plus a new dropdown smoke covering--colors/--highlightend-to-end against the upstreamhighlight_colors/enable_highlightfields