mirror of
https://github.com/larksuite/cli.git
synced 2026-07-11 11:44:07 +08:00
feat(sheets): make validation errors prescriptive for hot failure modes
Driven by the edit-eval-extra-35Q reports: ~70% of lark-cli sheets
errors were missing-required / JSON-shape / wrong-value classes whose
messages said what broke but not how to fix it, pushing agents into
--help / --print-schema probe loops.
- composite JSON shape errors inline a compact skeleton auto-generated
from the schema (e.g. --cells -> [[{"value": ...}]]) when the type
mismatch is shallow container confusion
- +batch-update: missing 'shortcut' shows the entry template; a
disallowed shortcut inlines the full allow-list; exceeding the
100-op cap says how many batches to split into; sub-op translator
failures append the shortcut's complete input-key contract
- +table-put: dtypes/formats keys that miss every column call out the
A1-letter habit and inline the declared column names; empty cells in
a date-typed column name the three ways out
- schema enum errors suggest across casing, vocabulary aliases, and
edit distance
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
package sheets
|
||||
|
||||
import (
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -206,6 +207,54 @@ var batchOpDispatch = map[string]batchOpMapping{
|
||||
"+float-image-delete": {"manage_float_image_object", objDeleteTranslate(floatImageDeleteSpec)},
|
||||
}
|
||||
|
||||
// allowedBatchShortcuts lists every shortcut accepted inside +batch-update,
|
||||
// sorted, for the not-allowed error hint.
|
||||
func allowedBatchShortcuts() []string {
|
||||
out := make([]string, 0, len(batchOpDispatch))
|
||||
for sc := range batchOpDispatch {
|
||||
out = append(out, sc)
|
||||
}
|
||||
sort.Strings(out)
|
||||
return out
|
||||
}
|
||||
|
||||
// subOpInputContract renders one shortcut's complete sub-op key vocabulary
|
||||
// (wire-style underscore names) for the translator-failure hint: required
|
||||
// flags are marked, the sheet selector pair collapses to a choose-one, and
|
||||
// spreadsheet locators are omitted (reserved for the batch top level).
|
||||
// Returns "" for shortcuts without a flag-defs entry.
|
||||
func subOpInputContract(sc string) string {
|
||||
defs, _ := loadFlagDefs()
|
||||
spec, ok := defs[sc]
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
idFlag, nameFlag := sheetSelectorFlagsForSubOp(sc)
|
||||
var keys []string
|
||||
sheetSelector := ""
|
||||
for _, df := range spec.Flags {
|
||||
if df.Kind == "system" || df.Hidden {
|
||||
continue
|
||||
}
|
||||
switch df.Name {
|
||||
case "url", "spreadsheet-token":
|
||||
continue // reserved: supplied by +batch-update top level
|
||||
case idFlag, nameFlag:
|
||||
sheetSelector = strings.ReplaceAll(idFlag, "-", "_") + "|" + strings.ReplaceAll(nameFlag, "-", "_") + " (choose one)"
|
||||
continue
|
||||
}
|
||||
key := strings.ReplaceAll(df.Name, "-", "_")
|
||||
if df.Required == "required" {
|
||||
key += " (required)"
|
||||
}
|
||||
keys = append(keys, key)
|
||||
}
|
||||
if sheetSelector != "" {
|
||||
keys = append([]string{sheetSelector}, keys...)
|
||||
}
|
||||
return strings.Join(keys, ", ")
|
||||
}
|
||||
|
||||
// rejectLocalImageInBatch blocks the local-file --image source inside
|
||||
// +batch-update: a batch sub-op has no upload phase, so the file could not be
|
||||
// turned into a file_token. Callers must pass --image-token / --image-uri.
|
||||
@@ -271,7 +320,8 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte
|
||||
}
|
||||
scRaw, present := op["shortcut"]
|
||||
if !present {
|
||||
return nil, sheetsValidationForFlag("operations", "operations[%d]: 'shortcut' field is required", index)
|
||||
return nil, sheetsValidationForFlag("operations", "operations[%d]: 'shortcut' field is required", index).
|
||||
WithHint(`each entry must look like {"shortcut":"+cells-set","input":{"sheet_name":"…","range":"A1:B2","cells":[[…]]}} — input uses the shortcut's own flag names`)
|
||||
}
|
||||
sc, ok := scRaw.(string)
|
||||
if !ok || sc == "" {
|
||||
@@ -279,13 +329,15 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte
|
||||
}
|
||||
mapping, ok := batchOpDispatch[sc]
|
||||
if !ok {
|
||||
// Inline the full allow-list: an agent that guessed a read op or a
|
||||
// fan-out wrapper can pick the right shortcut immediately instead of
|
||||
// spending a --print-schema round trip on the operations enum.
|
||||
return nil, sheetsValidationForFlag(
|
||||
"operations",
|
||||
"operations[%d]: shortcut %q not allowed in +batch-update "+
|
||||
"(read ops / fan-out wrappers like +batch-update / +cells-batch-set-style / +cells-batch-clear / +dropdown-{update,delete} are excluded; "+
|
||||
"run `lark-cli sheets +batch-update --print-schema --flag-name operations` to see the full enum)",
|
||||
"(read ops / fan-out wrappers like +batch-update / +cells-batch-set-style / +cells-batch-clear / +dropdown-{update,delete} are excluded)",
|
||||
index, sc,
|
||||
)
|
||||
).WithHint("allowed shortcuts: %s", strings.Join(allowedBatchShortcuts(), ", "))
|
||||
}
|
||||
inputRaw, hasInput := op["input"]
|
||||
var input map[string]interface{}
|
||||
@@ -333,7 +385,14 @@ func translateBatchOp(raw interface{}, token string, index int) (map[string]inte
|
||||
sheetName := strings.TrimSpace(fv.Str(sheetNameFlag))
|
||||
body, err := mapping.translate(fv, token, sheetID, sheetName)
|
||||
if err != nil {
|
||||
return nil, sheetsValidationForFlag("operations", "operations[%d] (%s): %v", index, sc, err)
|
||||
// The inner error names one problem at a time (first missing flag);
|
||||
// the hint lists the sub-op's complete key contract so an agent fixes
|
||||
// every gap in a single retry instead of iterating flag by flag.
|
||||
verr := sheetsValidationForFlag("operations", "operations[%d] (%s): %v", index, sc, err)
|
||||
if contract := subOpInputContract(sc); contract != "" {
|
||||
verr = verr.WithHint("%s input keys: %s", sc, contract)
|
||||
}
|
||||
return nil, verr
|
||||
}
|
||||
return map[string]interface{}{
|
||||
"tool_name": mapping.mcpToolName,
|
||||
@@ -354,7 +413,9 @@ func translateBatchOperations(rawOps []interface{}, token string) ([]interface{}
|
||||
return nil, sheetsValidationForFlag("operations", "--operations must be a non-empty JSON array")
|
||||
}
|
||||
if len(rawOps) > maxBatchOperations {
|
||||
return nil, sheetsValidationForFlag("operations", "--operations accepts at most %d entries; got %d", maxBatchOperations, len(rawOps))
|
||||
batches := (len(rawOps) + maxBatchOperations - 1) / maxBatchOperations
|
||||
return nil, sheetsValidationForFlag("operations", "--operations accepts at most %d entries; got %d", maxBatchOperations, len(rawOps)).
|
||||
WithHint("split the operations into %d separate +batch-update calls of at most %d entries each", batches, maxBatchOperations)
|
||||
}
|
||||
out := make([]interface{}, 0, len(rawOps))
|
||||
for i, raw := range rawOps {
|
||||
|
||||
@@ -5,6 +5,7 @@ package sheets
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
@@ -97,11 +98,22 @@ func validateValueAgainstSchema(fv flagView, name string, value interface{}) err
|
||||
// Composite-JSON shape errors (e.g. +cells-set --cells, chart
|
||||
// --properties) are the highest-frequency usage-layer failure for
|
||||
// sheets, and agents often burn several retries guessing the shape.
|
||||
// Point them straight at --print-schema, which dumps the exact JSON
|
||||
// Schema for this (command, flag) pair. The hint is always actionable:
|
||||
// reaching this branch means entry[name] resolved a schema from the
|
||||
// embedded index, and --print-schema reads that same index, so the
|
||||
// suggested command is guaranteed to print it.
|
||||
// A shallow type mismatch means the caller misremembered the overall
|
||||
// container shape (the classic {"cells": ...} wrapper around what
|
||||
// must be a bare 2D array), so inline a skeleton of the expected
|
||||
// shape — that fixes the retry without a --print-schema round trip.
|
||||
// Deeper failures keep the --print-schema pointer, which dumps the
|
||||
// exact JSON Schema for this (command, flag) pair; reaching this
|
||||
// branch means entry[name] resolved a schema from the embedded
|
||||
// index, so the suggested command is guaranteed to print it.
|
||||
var tm *typeMismatchError
|
||||
if errors.As(vErr, &tm) && pathDepth(tm.path) <= skeletonPathDepthLimit {
|
||||
if sk := schemaSkeleton(&schema, skeletonMaxDepth); sk != "" {
|
||||
return sheetsValidationForFlag(name,
|
||||
"--%s: %s; expected shape: %s (run `lark-cli sheets %s --print-schema --flag-name %s` for the full JSON Schema)",
|
||||
name, vErr.Error(), sk, command, name).WithCause(vErr)
|
||||
}
|
||||
}
|
||||
return sheetsValidationForFlag(name,
|
||||
"--%s: %s; run `lark-cli sheets %s --print-schema --flag-name %s` to see the expected JSON Schema",
|
||||
name, vErr.Error(), command, name).WithCause(vErr)
|
||||
@@ -243,7 +255,7 @@ func validateAgainstSchema(value interface{}, schema *schemaProperty, path strin
|
||||
|
||||
if schema.Type != "" {
|
||||
if !matchesJSONType(value, schema.Type) {
|
||||
return fmt.Errorf("%sexpected type %q, got %q", pathPrefix(path), schema.Type, jsType(value)) //nolint:forbidigo // intermediate error; validateFlagAgainstSchema wraps it into a typed flag validation error with a --print-schema hint
|
||||
return &typeMismatchError{path: path, expected: schema.Type, got: jsType(value)}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -279,7 +291,7 @@ func validateAgainstSchema(value interface{}, schema *schemaProperty, path strin
|
||||
if !matched {
|
||||
msg := fmt.Sprintf("%svalue %s is not in enum %s",
|
||||
pathPrefix(path), formatJSONValue(value), formatEnum(schema.Enum))
|
||||
if hint := suggestEnumMatch(value, schema.Enum); hint != "" {
|
||||
if hint := suggestEnumForError(value, schema.Enum); hint != "" {
|
||||
msg += fmt.Sprintf(` (did you mean %q?)`, hint)
|
||||
}
|
||||
return fmt.Errorf("%s", msg) //nolint:forbidigo // intermediate error; validateFlagAgainstSchema wraps it into a typed flag validation error with a --print-schema hint
|
||||
@@ -388,6 +400,126 @@ func validateAgainstSchema(value interface{}, schema *schemaProperty, path strin
|
||||
return nil
|
||||
}
|
||||
|
||||
// typeMismatchError is the type-check branch of validateAgainstSchema
|
||||
// as a typed error, so validateValueAgainstSchema can recognize shape
|
||||
// confusion (vs. deep value errors) and inline a skeleton of the
|
||||
// expected shape. Error() keeps the exact legacy wording.
|
||||
type typeMismatchError struct {
|
||||
path string
|
||||
expected string
|
||||
got string
|
||||
}
|
||||
|
||||
func (e *typeMismatchError) Error() string {
|
||||
return fmt.Sprintf("%sexpected type %q, got %q", pathPrefix(e.path), e.expected, e.got)
|
||||
}
|
||||
|
||||
// pathDepth counts how many levels below the flag root a JSON path
|
||||
// points at: "" → 0, "[0]" → 1, "[0][3]" → 2, "[0][3].value" → 3,
|
||||
// "legend" → 1, "snapshot.axes" → 2. Every "[" and "." starts a new
|
||||
// segment; a leading bare key (no bracket) is one segment of its own.
|
||||
func pathDepth(path string) int {
|
||||
depth := strings.Count(path, "[") + strings.Count(path, ".")
|
||||
if path != "" && path[0] != '[' {
|
||||
depth++
|
||||
}
|
||||
return depth
|
||||
}
|
||||
|
||||
// Skeleton rendering bounds: a mismatch at depth ≤ 2 is container-shape
|
||||
// confusion worth a skeleton; deeper mismatches are value-level and the
|
||||
// full schema pointer serves better. The skeleton itself stops after
|
||||
// four levels and eight keys per object so it stays one line; a wide
|
||||
// object (> skeletonWideObject keys) collapses its children to type
|
||||
// placeholders so every key stays visible instead of the first branch
|
||||
// eating the whole line.
|
||||
const (
|
||||
skeletonPathDepthLimit = 2
|
||||
skeletonMaxDepth = 4
|
||||
skeletonMaxKeys = 8
|
||||
skeletonWideObject = 2
|
||||
)
|
||||
|
||||
// schemaSkeleton renders a compact single-line sketch of the shape a
|
||||
// schema expects, e.g. [[{"value": …, "formula": "…", …}]] for
|
||||
// +cells-set --cells. Required keys come first, then alphabetical,
|
||||
// capped at skeletonMaxKeys with a trailing … marker. Values render as
|
||||
// their type placeholder; enum strings show the first allowed value.
|
||||
func schemaSkeleton(s *schemaProperty, depth int) string {
|
||||
if s == nil {
|
||||
return "…"
|
||||
}
|
||||
if len(s.OneOf) > 0 && s.Type == "" {
|
||||
return schemaSkeleton(s.OneOf[0], depth)
|
||||
}
|
||||
switch s.Type {
|
||||
case "array":
|
||||
if depth <= 0 {
|
||||
return "[…]"
|
||||
}
|
||||
return "[" + schemaSkeleton(s.Items, depth-1) + "]"
|
||||
case "object":
|
||||
if depth <= 0 || len(s.Properties) == 0 {
|
||||
return "{…}"
|
||||
}
|
||||
keys := skeletonKeys(s)
|
||||
childDepth := depth - 1
|
||||
if len(s.Properties) > skeletonWideObject {
|
||||
childDepth = 0
|
||||
}
|
||||
parts := make([]string, 0, len(keys)+1)
|
||||
for _, k := range keys {
|
||||
parts = append(parts, fmt.Sprintf("%q: %s", k, schemaSkeleton(s.Properties[k], childDepth)))
|
||||
}
|
||||
if len(s.Properties) > len(keys) {
|
||||
parts = append(parts, "…")
|
||||
}
|
||||
return "{" + strings.Join(parts, ", ") + "}"
|
||||
case "string":
|
||||
if len(s.Enum) > 0 {
|
||||
return formatJSONValue(s.Enum[0])
|
||||
}
|
||||
return `"…"`
|
||||
case "number", "integer":
|
||||
return "0"
|
||||
case "boolean":
|
||||
return "false"
|
||||
}
|
||||
return "…"
|
||||
}
|
||||
|
||||
// skeletonKeys picks which object keys a skeleton shows: required keys
|
||||
// first (schema order), then remaining keys alphabetically, capped at
|
||||
// skeletonMaxKeys.
|
||||
func skeletonKeys(s *schemaProperty) []string {
|
||||
keys := make([]string, 0, skeletonMaxKeys)
|
||||
seen := make(map[string]struct{}, skeletonMaxKeys)
|
||||
for _, k := range s.Required {
|
||||
if _, ok := s.Properties[k]; !ok {
|
||||
continue
|
||||
}
|
||||
if len(keys) == skeletonMaxKeys {
|
||||
return keys
|
||||
}
|
||||
keys = append(keys, k)
|
||||
seen[k] = struct{}{}
|
||||
}
|
||||
rest := make([]string, 0, len(s.Properties))
|
||||
for k := range s.Properties {
|
||||
if _, dup := seen[k]; !dup {
|
||||
rest = append(rest, k)
|
||||
}
|
||||
}
|
||||
sort.Strings(rest)
|
||||
for _, k := range rest {
|
||||
if len(keys) == skeletonMaxKeys {
|
||||
break
|
||||
}
|
||||
keys = append(keys, k)
|
||||
}
|
||||
return keys
|
||||
}
|
||||
|
||||
func matchesJSONType(value interface{}, expected string) bool {
|
||||
switch expected {
|
||||
case "object":
|
||||
@@ -473,25 +605,48 @@ func joinFormatted(values []interface{}) string {
|
||||
return strings.Join(parts, ", ")
|
||||
}
|
||||
|
||||
// suggestEnumMatch returns a "did you mean" candidate when the user's
|
||||
// value differs from an allowed enum entry only in casing — the most
|
||||
// common real-world mistake ("SUM" vs "sum", "True" vs "true"). The
|
||||
// match is restricted to strings; non-string enums (numbers, etc.)
|
||||
// don't have a casing notion. Returns "" when no near-miss exists.
|
||||
// suggestEnumMatch returns the canonical enum entry when the user's
|
||||
// value unambiguously means one — casing ("SUM" vs "sum", "True" vs
|
||||
// "true") or a cross-vocabulary alias (CSS "center" for Lark's vertical
|
||||
// "middle"). Callers auto-apply the result, so it must stay restricted
|
||||
// to unambiguous matches (edit-distance guesses belong in
|
||||
// suggestEnumForError only). Non-string values have no vocabulary
|
||||
// notion. Returns "" when no unambiguous match exists.
|
||||
func suggestEnumMatch(value interface{}, values []interface{}) string {
|
||||
s, ok := value.(string)
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
lower := strings.ToLower(s)
|
||||
canon := canonicalEnumValue(s, stringEnumEntries(values))
|
||||
if canon == "" || canon == s { // skip exact-equal (already would have matched).
|
||||
return ""
|
||||
}
|
||||
return canon
|
||||
}
|
||||
|
||||
// stringEnumEntries extracts the string members of a JSON-schema enum
|
||||
// list (mixed-type enums keep only their string entries).
|
||||
func stringEnumEntries(values []interface{}) []string {
|
||||
out := make([]string, 0, len(values))
|
||||
for _, v := range values {
|
||||
if vs, ok := v.(string); ok && strings.ToLower(vs) == lower {
|
||||
if vs != s { // skip exact-equal (already would have matched).
|
||||
return vs
|
||||
}
|
||||
if vs, ok := v.(string); ok {
|
||||
out = append(out, vs)
|
||||
}
|
||||
}
|
||||
return ""
|
||||
return out
|
||||
}
|
||||
|
||||
// suggestEnumForError picks the "did you mean" candidate for an enum
|
||||
// error message. Unlike suggestEnumMatch (whose result is auto-applied,
|
||||
// so it must stay unambiguous), this one may also draw on edit distance
|
||||
// — the suggestion is only prose, the user still has to re-issue the
|
||||
// value explicitly.
|
||||
func suggestEnumForError(value interface{}, values []interface{}) string {
|
||||
s, ok := value.(string)
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
return closestEnumValue(s, stringEnumEntries(values))
|
||||
}
|
||||
|
||||
func pathPrefix(path string) string {
|
||||
|
||||
@@ -360,6 +360,142 @@ func TestValidateInputAgainstSchema_RealEnumCaseNormalized(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateAgainstSchema_EnumAliasNormalized pins the cross-vocabulary
|
||||
// auto-fix: CSS-habit "center" for a vertical alignment unambiguously means
|
||||
// Lark's "middle", so the payload is normalized in place and the call
|
||||
// proceeds — same treatment as the "SUM" vs "sum" casing class.
|
||||
func TestValidateAgainstSchema_EnumAliasNormalized(t *testing.T) {
|
||||
t.Parallel()
|
||||
schema := parseSchema(t, `{
|
||||
"type":"object",
|
||||
"properties":{"vertical_alignment":{"type":"string","enum":["top","middle","bottom"]}}
|
||||
}`)
|
||||
obj := map[string]interface{}{"vertical_alignment": "center"}
|
||||
if err := validateAgainstSchema(obj, schema, ""); err != nil {
|
||||
t.Fatalf("center should normalize to middle and pass, got: %v", err)
|
||||
}
|
||||
if got := obj["vertical_alignment"]; got != "middle" {
|
||||
t.Errorf("vertical_alignment = %q, want normalized to %q", got, "middle")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateAgainstSchema_EnumTypoSuggestedNotApplied pins the auto-apply
|
||||
// boundary on the error path: an edit-distance typo stays an error with a
|
||||
// "did you mean" suggestion, never a silent rewrite.
|
||||
func TestValidateAgainstSchema_EnumTypoSuggestedNotApplied(t *testing.T) {
|
||||
t.Parallel()
|
||||
schema := parseSchema(t, `{
|
||||
"type":"object",
|
||||
"properties":{"order":{"type":"string","enum":["asc","desc"]}}
|
||||
}`)
|
||||
obj := map[string]interface{}{"order": "ascc"}
|
||||
err := validateAgainstSchema(obj, schema, "")
|
||||
if err == nil {
|
||||
t.Fatal("typo must be rejected")
|
||||
}
|
||||
if !strings.Contains(err.Error(), `did you mean "asc"?`) {
|
||||
t.Errorf("enum error should suggest asc for the typo, got %q", err.Error())
|
||||
}
|
||||
if got := obj["order"]; got != "ascc" {
|
||||
t.Errorf("typo must not be rewritten, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateValueAgainstSchema_ShapeSkeletonOnShallowTypeMismatch
|
||||
// pins the highest-frequency eval failure: passing an object where
|
||||
// --cells expects a 2D array must inline a skeleton of the expected
|
||||
// shape (with the "value" key visible) so an agent fixes the retry
|
||||
// without a --print-schema round trip.
|
||||
func TestValidateValueAgainstSchema_ShapeSkeletonOnShallowTypeMismatch(t *testing.T) {
|
||||
t.Parallel()
|
||||
fv := mapFlagView{command: "+cells-set"}
|
||||
err := validateValueAgainstSchema(fv, "cells",
|
||||
map[string]interface{}{"cells": []interface{}{}})
|
||||
if err == nil {
|
||||
t.Fatal("object where array expected must fail")
|
||||
}
|
||||
msg := err.Error()
|
||||
for _, want := range []string{`expected type "array", got "object"`, "expected shape: [[{", `"value"`} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Errorf("error should contain %q, got %q", want, msg)
|
||||
}
|
||||
}
|
||||
|
||||
// Deep value-level mismatch keeps the plain --print-schema pointer
|
||||
// (a whole-shape skeleton would not address the actual problem).
|
||||
deep := []interface{}{[]interface{}{
|
||||
map[string]interface{}{"note": 12.5},
|
||||
}}
|
||||
err = validateValueAgainstSchema(fv, "cells", deep)
|
||||
if err == nil {
|
||||
t.Fatal("wrong type for note must fail")
|
||||
}
|
||||
if strings.Contains(err.Error(), "expected shape:") {
|
||||
t.Errorf("deep mismatch should not inline a skeleton, got %q", err.Error())
|
||||
}
|
||||
if !strings.Contains(err.Error(), "--print-schema") {
|
||||
t.Errorf("deep mismatch should keep the --print-schema pointer, got %q", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestPathDepth(t *testing.T) {
|
||||
t.Parallel()
|
||||
cases := []struct {
|
||||
path string
|
||||
want int
|
||||
}{
|
||||
{"", 0},
|
||||
{"[0]", 1},
|
||||
{"[0][3]", 2},
|
||||
{"[0][3].value", 3},
|
||||
{"legend", 1},
|
||||
{"snapshot.axes", 2},
|
||||
}
|
||||
for _, c := range cases {
|
||||
if got := pathDepth(c.path); got != c.want {
|
||||
t.Errorf("pathDepth(%q) = %d, want %d", c.path, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSchemaSkeleton(t *testing.T) {
|
||||
t.Parallel()
|
||||
schema := parseSchema(t, `{
|
||||
"type":"array",
|
||||
"items":{
|
||||
"type":"array",
|
||||
"items":{
|
||||
"type":"object",
|
||||
"properties":{
|
||||
"value":{},
|
||||
"formula":{"type":"string"},
|
||||
"align":{"type":"string","enum":["top","middle","bottom"]},
|
||||
"styles":{"type":"object","properties":{"bold":{"type":"boolean"}}}
|
||||
}
|
||||
}
|
||||
}
|
||||
}`)
|
||||
got := schemaSkeleton(schema, skeletonMaxDepth)
|
||||
// Wide object (>2 keys) collapses nested containers to placeholders;
|
||||
// enum strings surface their first allowed value.
|
||||
want := `[[{"align": "top", "formula": "…", "styles": {…}, "value": …}]]`
|
||||
if got != want {
|
||||
t.Errorf("skeleton = %s, want %s", got, want)
|
||||
}
|
||||
|
||||
narrow := parseSchema(t, `{
|
||||
"type":"object",
|
||||
"required":["sheets"],
|
||||
"properties":{"sheets":{"type":"array","items":{"type":"object","properties":{"name":{"type":"string"}}}}}
|
||||
}`)
|
||||
got = schemaSkeleton(narrow, skeletonMaxDepth)
|
||||
// Narrow object (≤2 keys) keeps descending so the inner shape shows.
|
||||
want = `{"sheets": [{"name": "…"}]}`
|
||||
if got != want {
|
||||
t.Errorf("narrow skeleton = %s, want %s", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateAgainstSchema_NilSchemaSafe pins the defensive
|
||||
// `if schema == nil { return nil }` guard. Current production callers
|
||||
// always hand validator a real schema, but the guard means future
|
||||
|
||||
@@ -5,6 +5,7 @@ package sheets
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -419,6 +420,94 @@ func TestBatchUpdate_TranslatorRejects(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestBatchUpdate_PrescriptiveHints pins the recovery hints that ride on the
|
||||
// highest-frequency batch failures, so an agent can repair its payload in a
|
||||
// single retry without --help / --print-schema round trips.
|
||||
func TestBatchUpdate_PrescriptiveHints(t *testing.T) {
|
||||
t.Parallel()
|
||||
cases := []struct {
|
||||
name string
|
||||
opsJSON string
|
||||
wantMatch string
|
||||
wantInHint []string
|
||||
}{
|
||||
{
|
||||
name: "missing shortcut gets entry template",
|
||||
opsJSON: `[{"input":{"range":"A1"}}]`,
|
||||
wantMatch: "'shortcut' field is required",
|
||||
wantInHint: []string{`{"shortcut":"+cells-set"`, `"input"`},
|
||||
},
|
||||
{
|
||||
name: "disallowed shortcut lists the allow-list inline",
|
||||
opsJSON: `[{"shortcut":"+cells-batch-set-style","input":{}}]`,
|
||||
wantMatch: "not allowed in +batch-update",
|
||||
wantInHint: []string{"allowed shortcuts:", "+cells-set-style", "+range-copy"},
|
||||
},
|
||||
{
|
||||
name: "translator failure lists full key contract",
|
||||
opsJSON: `[{"shortcut":"+dim-insert","input":{"sheet_name":"s"}}]`,
|
||||
wantMatch: "--position is required",
|
||||
wantInHint: []string{"+dim-insert input keys:", "sheet_id|sheet_name (choose one)", "position (required)", "count (required)", "inherit_style"},
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, _, err := runShortcutCapturingErr(t, BatchUpdate, []string{
|
||||
"--url", testURL,
|
||||
"--operations", tc.opsJSON,
|
||||
"--yes",
|
||||
"--dry-run",
|
||||
})
|
||||
ve := requireValidation(t, err, tc.wantMatch)
|
||||
for _, want := range tc.wantInHint {
|
||||
if !strings.Contains(ve.Hint, want) {
|
||||
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestTranslateBatchOperations_OverLimitSplitHint pins the split
|
||||
// prescription on the 100-entry cap: the hint must say how many batches
|
||||
// the caller should re-issue.
|
||||
func TestTranslateBatchOperations_OverLimitSplitHint(t *testing.T) {
|
||||
t.Parallel()
|
||||
ops := make([]interface{}, 185)
|
||||
for i := range ops {
|
||||
ops[i] = map[string]interface{}{"shortcut": "+cells-set", "input": map[string]interface{}{}}
|
||||
}
|
||||
_, err := translateBatchOperations(ops, "shtcnX")
|
||||
ve := requireValidation(t, err, "accepts at most 100 entries; got 185")
|
||||
for _, want := range []string{"2 separate +batch-update calls", "at most 100 entries each"} {
|
||||
if !strings.Contains(ve.Hint, want) {
|
||||
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSubOpInputContract pins the contract line derivation from flag-defs:
|
||||
// reserved spreadsheet locators are omitted, the sheet selector collapses
|
||||
// to a choose-one, and required flags are marked.
|
||||
func TestSubOpInputContract(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := subOpInputContract("+dim-insert")
|
||||
for _, want := range []string{"sheet_id|sheet_name (choose one)", "position (required)", "count (required)", "inherit_style"} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("contract should contain %q, got %q", want, got)
|
||||
}
|
||||
}
|
||||
for _, banned := range []string{"url", "spreadsheet_token", "dry_run"} {
|
||||
if strings.Contains(got, banned) {
|
||||
t.Errorf("contract must not expose %q, got %q", banned, got)
|
||||
}
|
||||
}
|
||||
if got := subOpInputContract("+no-such-shortcut"); got != "" {
|
||||
t.Errorf("unknown shortcut should yield empty contract, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBatchUpdate_DimFreezeInjectsFreeze covers the static-freeze-only
|
||||
// path: +dim-freeze always injects operation=freeze (count==0 unfreeze
|
||||
// path of the single shortcut is intentionally not supported in batch).
|
||||
|
||||
@@ -317,17 +317,52 @@ func (in *tableSheetIn) normalize(idx int) (tableSheetSpec, error) {
|
||||
// compare against the canonical set.
|
||||
for k := range in.Dtypes {
|
||||
if !seenCol[k] {
|
||||
return tableSheetSpec{}, common.ValidationErrorf("--sheets[%d] %q: dtypes references unknown column %q", idx, in.Name, k)
|
||||
return tableSheetSpec{}, common.ValidationErrorf("--sheets[%d] %q: dtypes references unknown column %q", idx, in.Name, k).
|
||||
WithHint("%s", columnKeyHint("dtypes", k, in.Columns))
|
||||
}
|
||||
}
|
||||
for k := range in.Formats {
|
||||
if !seenCol[k] {
|
||||
return tableSheetSpec{}, common.ValidationErrorf("--sheets[%d] %q: formats references unknown column %q", idx, in.Name, k)
|
||||
return tableSheetSpec{}, common.ValidationErrorf("--sheets[%d] %q: formats references unknown column %q", idx, in.Name, k).
|
||||
WithHint("%s", columnKeyHint("formats", k, in.Columns))
|
||||
}
|
||||
}
|
||||
return spec, nil
|
||||
}
|
||||
|
||||
// columnKeyHint explains a dtypes/formats key that matched no column. The
|
||||
// dominant failure is Excel habit — keying by column letter (A/B/AA) instead
|
||||
// of the column name — so call that out explicitly; either way, inline the
|
||||
// declared column names so the retry needs no second look at the payload.
|
||||
func columnKeyHint(field, key string, columns []string) string {
|
||||
shown := columns
|
||||
const maxShown = 12
|
||||
suffix := ""
|
||||
if len(shown) > maxShown {
|
||||
shown = shown[:maxShown]
|
||||
suffix = ", …"
|
||||
}
|
||||
list := `"` + strings.Join(shown, `", "`) + `"` + suffix
|
||||
if isColumnLetterKey(key) {
|
||||
return fmt.Sprintf("%s keys must be column names from `columns`, not A1-style column letters; this sheet's columns: %s", field, list)
|
||||
}
|
||||
return fmt.Sprintf("%s keys must exactly match a name in `columns`: %s", field, list)
|
||||
}
|
||||
|
||||
// isColumnLetterKey reports whether key looks like an A1-style column letter
|
||||
// (A, B, AA, …) rather than a real column name.
|
||||
func isColumnLetterKey(key string) bool {
|
||||
if key == "" || len(key) > 3 {
|
||||
return false
|
||||
}
|
||||
for _, r := range key {
|
||||
if r < 'A' || r > 'Z' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (p *tablePayload) validate() error {
|
||||
if len(p.Sheets) == 0 {
|
||||
return common.ValidationErrorf("--sheets: must contain at least one sheet")
|
||||
@@ -584,6 +619,12 @@ var excelEpoch = time.Date(1899, 12, 30, 0, 0, 0, 0, time.UTC)
|
||||
// parser still rejects it cleanly.
|
||||
func isoDateToSerial(s string) (int, error) {
|
||||
s = strings.TrimSpace(s)
|
||||
if s == "" {
|
||||
// Empty cells in a date-typed column are the classic header/total-row
|
||||
// clash with the column-wide dtype declaration; name the three ways
|
||||
// out so the caller does not have to guess what "bad format" means.
|
||||
return 0, fmt.Errorf("date column has an empty cell — drop the empty rows, fill real yyyy-mm-dd dates, or declare the column dtype as object (text)") //nolint:forbidigo // intermediate error; callers wrap it into a typed --sheets/--values validation error with row/column context
|
||||
}
|
||||
if i := strings.Index(s, "T"); i > 0 {
|
||||
s = s[:i]
|
||||
}
|
||||
|
||||
@@ -54,6 +54,67 @@ func TestTablePut_IsoDateToSerial(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestTablePut_EmptyDatePrescription pins the empty-cell branch: the error
|
||||
// must name the three ways out (drop rows / fill dates / object dtype)
|
||||
// instead of the generic "must be ISO" parse failure.
|
||||
func TestTablePut_EmptyDatePrescription(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, in := range []string{"", " "} {
|
||||
_, err := isoDateToSerial(in)
|
||||
if err == nil {
|
||||
t.Fatalf("isoDateToSerial(%q) should fail", in)
|
||||
}
|
||||
for _, want := range []string{"empty cell", "object (text)"} {
|
||||
if !strings.Contains(err.Error(), want) {
|
||||
t.Errorf("isoDateToSerial(%q) error should contain %q, got %q", in, want, err.Error())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestTablePut_ColumnKeyHint pins the dtypes/formats unknown-column hint:
|
||||
// A1-style letter keys get the Excel-habit callout, and the declared column
|
||||
// names ride inline either way.
|
||||
func TestTablePut_ColumnKeyHint(t *testing.T) {
|
||||
t.Parallel()
|
||||
cols := []string{"姓名", "出生日期"}
|
||||
got := columnKeyHint("dtypes", "A", cols)
|
||||
for _, want := range []string{"not A1-style column letters", `"姓名", "出生日期"`} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("letter-key hint should contain %q, got %q", want, got)
|
||||
}
|
||||
}
|
||||
got = columnKeyHint("formats", "出生 日期", cols)
|
||||
if strings.Contains(got, "A1-style") {
|
||||
t.Errorf("non-letter key must not get the letter callout, got %q", got)
|
||||
}
|
||||
if !strings.Contains(got, `"姓名", "出生日期"`) {
|
||||
t.Errorf("hint should inline column names, got %q", got)
|
||||
}
|
||||
|
||||
many := make([]string, 20)
|
||||
for i := range many {
|
||||
many[i] = fmt.Sprintf("col%02d", i)
|
||||
}
|
||||
got = columnKeyHint("dtypes", "X", many)
|
||||
if !strings.Contains(got, ", …") || strings.Contains(got, "col19") {
|
||||
t.Errorf("hint should truncate long column lists, got %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsColumnLetterKey(t *testing.T) {
|
||||
t.Parallel()
|
||||
cases := map[string]bool{
|
||||
"A": true, "Z": true, "AA": true, "ABC": true,
|
||||
"": false, "ABCD": false, "a": false, "A1": false, "姓名": false,
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := isColumnLetterKey(in); got != want {
|
||||
t.Errorf("isColumnLetterKey(%q) = %v, want %v", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestTablePut_BuildTypedCell(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user