feat(sheets): batch-update contract hardening and style-vocabulary acceptance#2028
Merged
xiongyuanwen-byted merged 16 commits intoJul 24, 2026
Merged
Conversation
Eval-driven fixes for the top +batch-update error clusters (137 errors across 7 eval batches, attribution in the optimization plan doc): - Reject unknown sub-op input keys with did-you-mean + full key contract instead of silently ignoring them (silent ignore surfaced as misleading 'missing required flag' errors — the largest cluster, ~35 hits). Habitual spellings are rewritten in place: camelCase -> snake_case, commandFlagAliases (new: size -> width/height on the resize pair, the pre-July vocabulary and the --styles protocol spelling, 15+ hits), single-entry ranges unwraps onto range. - Aggregate per-op validation errors into one pass (each op's first error) instead of fail-fast first-error-only; single-error batches keep the standalone-shaped error (contract tests unchanged). - Precheck cells matrix vs range locally: empty cells (prescribes +cells-clear) and row/column count mismatches no longer reach the server mid-batch. - Correct the atomicity story: execution is fail-fast without rollback (verified against live batches; docs promised a rollback that does not happen). Partial-failure errors now spell out that succeeded operations stay applied and prescribe resending only the failed tail, preventing double-apply on retry.
Agents stage generated payloads (batch operations JSON, CSV) in /tmp as a matter of course; the relative-to-cwd-only policy pushed every --operations @/tmp/ops.json through an extra python/stdin round trip (recurring friction cluster in eval traces). Scope is deliberately narrow: a new SafeTempAbsInputPath accepts an absolute READ path only when it resolves (symlinks included) under the canonical os.TempDir(), and only the @file expansion in cmdutil.ReadInputFile uses it. SafeInputPath stays strict — uploads, drive sync and the CI quality gates treat 'absolute paths rejected' as a load-bearing invariant. Absolute paths outside the temp dir now get a prescriptive error naming all three options (relative path, temp-dir path, stdin).
Batch-B of the +batch-update overhaul (attribution: ~73% of real batch
calls were pure formatting finishers hand-built as operations arrays).
- +styles-put: declarative visual spec for existing spreadsheets.
Reuses the workbook-create/table-put --styles parser (identical
vocabulary and aggregate-all-issues errors), expands client-side into
ONE atomic batch_update per spec: cell_merges -> cell_styles ->
row_sizes -> col_sizes -> freeze. Style stamps are safe to re-run.
Verified live: 6/6 sub-ops applied, frozen rows / merges / row
heights confirmed by read-back.
- freeze section added to the shared --styles pipeline ({rows, cols}),
so +workbook-create and +table-put gain it too — closes the one gap
that still forced a separate +dim-freeze call.
- +dim-delete --ranges: scattered row/column ranges in one atomic
batch, ordered DESCENDING so earlier deletions never shift later
indexes (the recurring failure of hand-built dim-delete batches);
same-dimension and non-overlap enforced, nesting inside
+batch-update rejected with a prescription.
- +cells-batch-set-style enters phase-1 deprecation: kept working,
docs point at +styles-put, an in-band note steers new usage.
Skill docs regenerated from sheet-skill-spec (new
lark-sheets-styles-put reference, three-way dispatch in guideline 6,
fail-fast-no-rollback wording).
…yles row/col_sizes size stays the canonical dimension key: it keeps row_sizes and col_sizes items shape-uniform (the array name already carries the dimension), it is what shipped with workbook-create/table-put and what models demonstrably converge on, and it matches the dimension-neutral precedent of comparable APIs. The Excel-vocabulary words are accepted silently only where unambiguous — height inside row_sizes, width inside col_sizes; the wrong dimension's word gets a targeted error instead of a rewrite, and giving both size and the alias is rejected. Shared parser, so +workbook-create / +table-put / +styles-put all gain it.
…he skill surface Regenerated from sheet-skill-spec: the deprecated command no longer appears anywhere in SKILL.md or the references (multi-range styling routes to +styles-put); its flag-defs entry is untouched, so the command and --help keep working for compatibility callers, with the in-band supersedence note steering them to +styles-put.
07-20 rerun attribution: the batch-update dispatch worked (calls 105->25,
errors 21->8; +styles-put adopted by 16/35 tasks) but +styles-put itself
hit a 56% stateful error rate — the redesign moved traffic from the flag
path onto the payload path, and the round-2 forgiveness layers (key
aliases, enum-value canonicalization) only existed on the flag path.
Both dominant clusters are fixed in the shared styles pipeline, so
+styles-put / +table-put / +workbook-create / typed --cells all gain it:
- border family folding (largest cluster, up to 88 issues in one retry):
borders/border objects, border_top..right objects, border_style/color/
weight scalars, and flattened border_<side>_<attr> keys all fold into
the canonical nested border_styles; border_style:"thin" reads as a
thin solid line (weight vocabulary in the style slot).
- enum VALUE canonicalization inside cell_styles (~10 server-side
round trips: vertical_alignment "center" -> "middle"), sourced from
the +cells-set-style flag enums; off-enum values now fail client-side
with a did-you-mean instead of failing the whole batch server-side.
- wrap family: wrap_text/text_wrap -> word_wrap, boolean -> enum.
- bare-string cell_merges entries read as {range, merge_type:all}.
- fore_color gets a prescription (openpyxl fgColor is the FILL color;
a silent pick could color the wrong thing).
Replayed the eval-failing payload shapes live: 2/2 applied.
…ntract tests
- {range, size} in row/col_sizes now means a pixel resize (type stays
for standard/auto) — the payload path matches the flag path, where
--width never required --type.
- Two closure tests turn the --styles acceptance surface into a locked
contract instead of open-ended patching:
* vocabulary parity — every +cells-set-style flag (iterated from
flag-defs) must be accepted verbatim by the payload path, so a
future flag can never again ship without payload-path support;
* prior corpus — every model spelling observed across the 07-08..07-20
eval batches must either normalize to canonical or produce a
targeted prescription; silent ignoring and bare rejection both fail
the suite. New eval finding -> add a corpus row -> fix -> locked.
Skill docs regenerated: the border shorthand ({style,weight,color} on
all four sides) is now the teaching form, border_styles demoted to
per-side differences; resize examples drop the type ceremony.
…coalescing, typed-cells style key Valid rerun (skill injection verified at ~31k chars/task): +styles-put stateful error rate fell 56.5% -> 34.3% and batch-update stayed at its post-dispatch low. Three residual clusters, all closed: - weight vocabulary in the FULL nested form's style slot (border_styles.<side>.style:"thin" — 8 tasks, the dominant residual; the earlier rewrite only covered the shorthand scalar path). Now normalized in expandBorderAllShorthand, the single border touchpoint shared by the flag, typed-cells and styles-payload paths. - per-row specs blowing the 100-op cap (184/203/861-op expansions): coalesceStyleStamps fuses identical-style entries into rectangles (vertical fixpoint merge on same column span, horizontal on same row span) before the cap — a declarative spec describes intent, execution shape is the CLI's to optimize. Cap message now also routes alternating-row banding to +cond-format-create. - typed --cells habitual keys (recurring server-side 900015206 in both reruns): cells[][].style object rewrites to cell_styles; cells[][].type gets a prescription instead of a server round trip. The content-in-styles message is also neutral now (was workbook-create-specific). Corpus + coalescing + typed-cells tests added; live replay: full-form thin accepted and 3 same-style rows fused, all applied.
…home Pure mechanical move, zero behavior change (locked by the acceptance contract tests). The acceptance layer had grown as an accretion across helpers.go and lark_sheet_workbook.go — deliberate design (one canonical form + wide acceptance, per the divergent-priors evidence), accidental placement. style_vocab.go now holds the whole subsystem — flag-path style builders, key aliases, enum-value canonicalization, border folding/normalization, typed-cells cell-object rewrites — under a header that states the design contract: rewrites must be unambiguous, ambiguity prescribes, silence and bare rejection are both bugs, closure is enforced by the parity + prior-corpus tests. helpers.go shrinks back to generic plumbing (818 -> 542 lines). The other two acceptance surfaces keep their own homes: cobra flag ergonomics in flag_ergonomics.go, batch sub-op key vocabulary in batch_op_dispatch.go.
The precheck deliberately skipped bare single-cell ranges when server behavior was unverified; the 07-21 rerun supplied the evidence (12 rows against range "A1" failing server-side with row count 1) — +cells-set has no anchor semantics, the strict match applies everywhere. Corpus updated accordingly.
…e sheet The tool requires a range but clips past-grid references and reports the clip in actual_range, so the CLI sends an over-wide whole-columns range (A:ZZZ) when --range is omitted: one call reads the entire sheet, no workbook-info pre-flight to size it first. Eval evidence: 'required flag(s) range not set' was the most-missed required flag on +csv-get (4 tasks in the 07-21 batch) — the models' intent was always 'read it all'. Blank --range now means the same as omitting it. Skill docs updated (quick-reference row, read-data full-read example, flag desc); round-3 backlog item A5.
github-actions
Bot
added
domain/ccm
|
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 Plus 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 |
xiongyuanwen-byted
force-pushed
the
feat/batch-update-optimization
branch
from
July 23, 2026 08:57
a1f0c64 to
bae732f
Compare
补齐 / 扩展 / 按原表格式续填时，查不到或无法确定的值一律留空 + 备注注明，禁止用推算 / 估算 / 凭空数据充数；原表已示范缺失值写法时照抄。 Synced from sheet-skill-spec (SoT).
07-21 evening batch (first run carrying the previous fixes — thin full-form / style/type keys / csv-get range all at zero): the corpus loop caught the next spelling permutations. bottom_border / bottom_border_style (side-first word order, alongside the border_bottom family already folded) and wrap_strategy (the Google Sheets API word) now normalize; three corpus rows lock them.
…ze the silent-alias admission bar The alias table was drifting toward per-permutation entries with fast- falling marginal value (first aliases covered 15+ errors each, the last ones 1-2). The asymmetry that matters: rejection is universal, aliases are per-word. So: - unknown style fields now reject with did-you-mean + the full canonical field list (this error had neither — the actual reason word-order permutations turned into multi-issue retry loops). Any future permutation costs one self-healing retry, zero new code, and corrects the whole session (silent aliases never correct the model in-session). - the acceptance-layer contract now states the admission bar for silent aliases: real external vocabularies only (Excel/openpyxl, CSS, Google Sheets API — a finite set), recurring across batches or ≥3 tasks, zero ambiguity. Permutations go to the universal rejection. Existing permutation aliases are grandfathered.
…one atomic call
The last compressible batch-update scenario: eval traces show 'fix all
broken formulas across ranges/sheets' (~6 calls / 3 tasks per batch)
still hand-assembled as +batch-update operations arrays. --writes takes
[{sheet_name|sheet_id, range, cells}, ...] (up to 100 items, cross-sheet)
and fans it into ONE atomic batch_update of set_cell_range ops.
Design decisions:
- the sheet selector LIVES IN EACH ITEM — no top-level fallback, no
precedence table; same convention models already learned from
+batch-update sub-ops and +styles-put items. A top-level
--sheet-name/--sheet-id with --writes is rejected with the fix.
- every item runs the exact standalone pipeline via a per-item flag
view: key vocabulary (camelCase, aliases, did-you-mean), the style
acceptance layer for inline cell_styles, matrix precheck, schema
validation; item errors aggregate so one retry fixes all.
- XOR with --range/--cells/--copy-to-range; top-level --allow-overwrite
propagates to items that don't override it.
- nesting inside +batch-update rejected (expands into its own batch).
- predictable prior handled: --styles on +cells-set now hints the
layering (range-level styling -> +styles-put; per-cell styles ride in
the cells objects) instead of a bare unknown-flag error.
Live smoke: value + formula regions in one call, 2/2 applied.
Expected effect: batch-update calls drop another ~30% to its
heterogeneous-atomic-chain steady state.
Version roll-up for the batch-update optimization series: cells-set --writes, universal did-you-mean on style fields, border word-order and wrap_strategy acceptance, editing rule #10, and optional +csv-get --range.
xiongyuanwen-byted
force-pushed
the
feat/batch-update-optimization
branch
from
July 23, 2026 08:57
bae732f to
d21cd1b
Compare
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
Eval-driven optimization pass over
+batch-updateand the style vocabulary, from the 137-error attribution batch. 16 commits, grouped by theme:+batch-update sub-op contract
sizeon the resize pair, single-entryranges) are rewritten in place; anything else errors with a did-you-mean plus the shortcut's full key contract (previously unknown keys were silently ignored and surfaced as misleading "missing required flag" errors — the top batch error cluster).+sheet-movesub-ops require explicitsheet_id/source_index/index(no mid-batch network lookups).@filepayload reads from the system temp dir (P0-6).Style vocabulary acceptance
+styles-putshortcut: post-hoc visual spec applied to an existing spreadsheet in one atomic batch;freezesupport in--styles;+dim-delete --ranges.wrap_strategy,height/widthas one-way aliases forsize, resizetypeoptional, universal did-you-mean on unknown style fields with a formalized silent-alias admission bar.+cells-set --writes: scattered multi-region writes in one atomic call (plural form rejected inside+batch-update, same as+dim-delete --ranges).Skill surface
+cells-batch-set-stylefully exits the skill surface;+csv-get --rangeoptional (whole-sheet read); editing rule fix(docs): normalize board_tokens in +create response for mermaid/whiteboard content #10 (never fabricate missing values); skill version bumped to 3.1.0.Rebase note
Rebased onto the current
feat/lark-sheets-develop, which had landed PR #2027 (aggregated validation errors + inline schema hints) in the same region. Resolution: kept #2027's landed error-aggregation implementation intranslateBatchOperationsand dropped this branch's duplicate (aggregateOpErrors); the two key-guard layers are complementary and both kept (wrappedSubOpInputKeysfrom #2027 runs before this branch'snormalizeSubOpInputKeys).Testing
go build ./...clean; fullgo test ./...passes except the pre-existing live-API e2eTestIM_ChatMessageWorkflowAsUser(IM, untouched by this branch).