codex/fix-cli-eval-functional
12
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 huwenjiao.hwjandClaude Opus 5
|
332b74e8ce |
refactor(sheet): split create-with-data and export-csv onto their own leaves
Both capabilities were added as flags on an existing leaf, and in both cases the leaf's published interface stopped describing what the command did: - `sheet create --values/--sheets/--styles` orchestrates create → probe → resolve default worksheet → write → read back → optional styles, yet the leaf still published `interface_mode: mcp` + `create_workspace_sheet`. - `sheet export --export-format csv` reads `get_range_as_csv` and never invokes `submit_export_job`, yet the leaf published `submit_export_job`. Each moves to its own command, declaring the interface it actually uses: `sheet create-with-data` is `composite` with a reviewed reason and no `interface_ref`; `sheet export-csv` is `mcp` + `get_range_as_csv`. Both are pinned in the interface-disposition contract test. `sheet create` and `sheet export` are restored byte-for-byte to main, so the compatibility gates see two `command_added` additions instead of four locked-field changes. The split also removes a user-visible trap: `--range` without `--export-format csv` used to be silently discarded and the whole workbook exported; the cross-format flags no longer exist, pinned by TestSheetExportAndExportCsvFlagsDoNotLeak. Drops the 8 now-stale mapping-ledger exclusions that covered the flags on `sheet.create_workspace_sheet` / `sheet.submit_export_job`, shrinking this branch's exclusion surface. Skill references and CHANGELOG follow the split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
huwenjiao.hwjandClaude Opus 5
|
8f135ecde6 |
fix(sheet): reject unknown and mistyped fields in border edge configs
parseBorderStyles read only style and a string color, so every other key and
any non-string color was silently dropped:
{"top":{"style":"solid","colour":"#f00"}} succeeded and drew a border with no
colour, and color: 123 did the same. That contradicts the unknown-key
rejection this PR applies to --sheets and --styles — a partially applied
style reported as success is harder to notice than an error.
Each edge now accepts only style/color, rejects near-miss spellings with the
canonical key, and fails when style or color is present with the wrong type
or empty. All three entry points (set-style --border-styles-json,
batch-set-style --ranges/--batch, and create --styles border_styles) share
parseBorderStyles, so one fix covers them; tests assert the rejection happens
before any MCP call on every path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
huwenjiao.hwjandClaude Opus 5
|
e026c754e0 |
fix(sheet): require EOF after the --values / --sheets JSON value
json.Decoder.Decode returns after one value, so `--values '[[1]] trailing'` was accepted as a valid matrix and the document got created anyway. A paste that ran long, leftover shell concatenation, or two JSON values glued together would silently drop the tail and still create a document the user never asked for — and creation cannot be rolled back atomically. Require EOF after the first value on both flags, following decodeOARequest. docs(sheet): document the fail-closed CSV export and --allow-truncated The skill references still claimed an oversized table is truncated with a warning on stderr, and omitted the flag. The command now aborts before writing anything when the server reports hasMore, so an agent relying on the skill would misread the result and had no way to learn how to opt in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
huwenjiao.hwjandClaude Opus 5
|
a097d57510 |
fix(sheet): validate the full --sheets contract before creating the document
sheet create promises that every structural check happens before the first
MCP request, but each sheet spec was only checked for object type and name:
columns/data/dtypes/formats/startCell were left to table_put, so a bad type
created and renamed the remote document first and failed at write time,
leaving behind a document the user never successfully asked for. Validate
every provided field against table_put's input contract up front, and reject
the malformed {"sheets":"bad"} wrapper instead of treating it as one spec.
Also fixed while auditing the same flow:
- unknown/misspelled keys are now rejected in both --sheets and --styles.
The server DTOs are fixed beans, so a stray "datas" was silently dropped
and the read-back probe landed on the header row: full data loss reported
as success. Near-miss spellings get the canonical key in the message.
- columns is required (the server requires it), non-blank and trim-unique;
dtypes/formats keys must resolve to a column, since the server looks them
up by trimmed name and silently ignores the rest.
- sheetId inside a spec is rejected: the document does not exist yet.
- the read-back probe now honours header:false, mode:append (a fresh sheet
appends at row 1, the startCell row is ignored) and $-absolute/lowercase
startCell refs, which the server accepts after uppercasing.
- --values cells must be scalars; a map used to be written as "map[a:1]".
- the 30000-cell and 2000000-char write limits are enforced locally.
Docs: the --styles top level only accepts snake_case (camelCase aliases are
inner-field only), and the read-back probe is not pinned to A1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
huwenjiao.hwj
|
eb73b944a1 |
fix(sheet): reject row/column ranges with trailing characters up front
parseRowColRange gates --styles row_sizes/col_sizes before the document is created. The row branch parsed with fmt.Sscanf(a, "%d", &r1), which consumes only the leading digits and does not require the whole token, so "1x:3" was silently accepted as row 1 and "2foo" as row 2. Such input passed pre-flight, then update_dimension was sent to the wrong row after the document and data had already been created — an unrollbackable wrong edit, the opposite of the gate's purpose. The row branch now parses with strconv.Atoi, which requires the entire token to be a valid integer. The column branch had the same class of hole via a different path: parseA1Cell appends "1" to the column token, so "A5" became "A51" and was accepted as column A. A new isAllLetters pre-check requires the column token to be non-empty and letters-only before parsing; genuine multi-letter columns like "AX" still pass. With that guarantee the subsequent parseA1Cell can no longer fail, so its now-dead error branch is removed. Adds trailing-character rejection cases to TestParseRowColRange: "1x:3", "2foo", "1 2:3" (rows), "A5:C", "A1", ":C" (columns), plus "AX:C" to confirm multi-letter columns remain valid. Changed-code coverage stays at 100%. |
||
|
huwenjiao.hwj
|
ecaa375be8 |
fix(sheet): validate merge range up front, fix startCell key, correct CHANGELOG
Three review P1s. 1. CHANGELOG no longer asserts a breaking Schema change this PR does not ship. sheet export / sheet create deliver main's mcp + interface_ref and schema-compatibility reports ok (0 changed fields), so the "declared as composite (breaking)" entry was false and is removed; the set-style entry drops the "breaking" framing (accepted as compatible by the reviewed mapping-exclusion carve-out); the duplicate ### Changed heading is merged. 2. firstNonEmptySheetSpecCell reads the start cell via pickStr(spec, "startCell", "start_cell"). Sheet specs are forwarded verbatim to table_put, whose wire fields are camelCase in this repo. The prior snake_case-only read meant a user passing the real startCell would have data written at the offset while the probe read A1 — a false "写入未生效" on a successful write. camelCase preferred, snake_case kept for tolerance. 3. planStyleOps now parses cell_merges range with parseA1Range, matching the cell_styles branch. planStyleOps is dry-run once before create_workspace_sheet as the up-front structural gate, so an invalid range like "not-a-range" is now rejected before any RPC instead of failing only at the final merge_cells call and leaving an unrollbackable partially-completed document. merge_cells' range contract is A1:B3-style, which parseA1Range covers (and it strips a Sheet1! prefix). Tests: cell-merges-invalid-range added to TestSheetCreateValidatesBeforeCreating Document (asserts calls == 0); TestFirstNonEmptySheetSpecCell covers both startCell and start_cell. Changed-code coverage stays at 100%; targeted sheet suites pass; CHANGELOG has a single Changed section with no false breaking claim. |
||
|
huwenjiao.hwj
|
dbecf23bc4 |
test(sheet): cover --sheets read-back error paths to reach 100% changed-code coverage
Adds coverage for the branches introduced by the per-sheet read-back: resolveSheetIDsByName's RPC-error and unparseable-response paths, the create --sheets path surfacing a list-fetch failure with the nodeId, and the single-value data row in sheetSpecGrid. Changed-code coverage back to 100%. |
||
|
huwenjiao.hwj
|
d73e199d97 |
fix(sheet): read back each sheet after --sheets table_put to catch silent data loss
The --values branch already reads back its first non-empty cell after writing, to defend against the new-document initialization race where a write returns success but the data does not land. The --sheets branch called table_put and reported success with no read-back, so the same race would let the command exit successfully while one or more sheets silently lost their initial data. After table_put, the --sheets branch now: - re-fetches get_all_sheets to build a name -> sheetId map (table_put reuses the renamed default sheet and auto-creates the rest by name), and - for every spec that actually has content, reads back its first expected non-empty cell and fails if the read-back is empty or the sheet is missing. firstNonEmptySheetSpecCell mirrors firstNonEmptyValuesCell: it treats columns as the header row followed by data rows, honours start_cell, and returns hasContent=false for a name-only spec so a legitimately empty sheet is not misreported as data loss. Failures carry the nodeId and point at sheet table-put for recovery, matching the --values branch's error shape. Tests: - TestSheetCreateWithSheetsVerifiesEachSheetLanded covers an empty read-back (errors, naming the sheet + nodeId + table-put), a sheet missing from the post-write listing, and a name-only sheet that must not trigger a read-back. - TestFirstNonEmptySheetSpecCell covers header/data origins, an empty first header cell, a start_cell offset, and content-less specs. - Existing --sheets tests updated for the added get_all_sheets + per-sheet read-back calls. |
||
|
huwenjiao.hwj
|
115dad3b82 |
fix(sheet): keep JSON integer literals intact through both data channels
P1 from automated review. --values decoded with plain json.Unmarshal, so every number became a float64 and integers beyond 2^53 were rounded before anything was written. The read-back only checks that the probe cell is non-empty, so the corruption was reported as a successful write. Order numbers and snowflake IDs are ordinary spreadsheet data. Measured before the fix: 1234567890123456789 -> 1234567890123456768 snowflake id, tail rewritten 12345678901234567890 -> 12345678901234567000 20-digit order number 9007199254740993 -> 9007199254740992 2^53+1 --sheets had the same defect, which the review did not mention: its records and data are forwarded verbatim to table_put, and the float64 round trip rewrote 1234567890123456789 as 1234567890123456800 before the request left the CLI. Both channels now decode with json.Decoder.UseNumber, and cellToString emits a json.Number through its String method so no float conversion happens on the way to CSV. --styles keeps plain Unmarshal on purpose: its numbers are font sizes and pixel dimensions, already constrained to int32 by pickNum, with no large-integer case. Tests assert the payload the CLI actually sends, not a recomputed decode: - TestSheetCreatePreservesLargeIntegerLiterals checks the csv argument of set_range_from_csv and the marshalled table_put arguments. Both halves also assert the rounded forms are absent, so removing UseNumber fails the test instead of passing on a lucky substring match. - TestCellToStringKeepsJSONNumberVerbatim covers large, negative, fractional and exponent literals, and keeps the existing float64 behaviour for other callers. The shared scriptedToolCaller keeps only the last call, and the write is the fourth of five, so the assertion needs an intermediate call. Rather than extend that shared helper, this adds a callRecorder local to this file: InitDeps takes the edition.ToolCaller interface, so embedding *scriptedToolCaller and overriding CallTool is enough. Changed-code coverage stays at 100.0000% (850 statements) per check-coverage-gate.sh --changed-only. |
||
|
huwenjiao.hwj
|
b8ac9810f4 |
fix(sheet): require unique --sheets names and cover the size error branches
Two things, both in the create-with-data path.
Unique worksheet names. parseCreateSheetSpecs accepted a --sheets payload with
repeated names, so table_put created several worksheets sharing one name. The
style tools locate a worksheet by "id or name", so --styles would then land on
whichever duplicate the server picked, and --styles runs after the document
already exists and cannot be rolled back. Duplicates are now refused before
anything is created, naming the first occurrence:
--sheets[1].name="一月" 与 --sheets[0] 重复；工作表名必须唯一，...
Case-only differences are still accepted: the server distinguishes them and the
CLI should not tighten that. --styles needs no equivalent check because it
already requires name equality with the corresponding --sheets entry, and those
are now unique.
Coverage. The previous commit added a precise pickNum error path to the standard
and auto branches of planSizes, but no test reached it: the existing cases used
an integer size, which stops at "不能同时给 size" before the numeric check runs.
The CI coverage gate therefore reported 99.7619% on changed code
(sheet_create_with_data.go:512-514 and :521-523). Two cases now drive
type=standard and type=auto with size 28.5.
That pair is not only about the percentage. It pins the error precedence: a
fractional size must report "size=28.5 必须是整数", which points at the field
actually written wrong, rather than the generic "不能同时给 size". Swapping the
two checks would make the message misleading and now fails the tests.
Changed-code coverage measured locally at 100.0000% (845/845 statements), with
no uncovered blocks in the diff against upstream/main.
Tests:
- TestParseCreateSheetSpecsRejectsDuplicateNames covers adjacent, non-adjacent
and {"sheets":[...]}-wrapped duplicates, plus three payloads that must pass.
- Three new cases in TestSheetCreateValidatesBeforeCreatingDocument
(sheets-duplicate-name and the two fractional sizes), each asserting calls == 0.
|
||
|
huwenjiao.hwj
|
c1a90c0194 |
fix(sheet): reject fractional and out-of-range --styles sizes instead of truncating
P2 from automated review. pickNum ran int(n) straight on the float64 that JSON decoding always produces, so font_size: 12.9 and row_sizes.size: 28.5 were silently rewritten to 12 and 28 and then executed as a valid configuration. Both the help text and the error messages state these fields must be positive integers, and --styles is a non-atomic sequence that cannot be rolled back, so truncation left a sheet that did not match what the caller asked for. Measured before the fix: font_size=12.9 -> emitted fontSize=12 size=28.5 -> emitted pixelSize=28 size=1e20 -> emitted pixelSize=9223372036854775807 The overflow case was worse than reported: int(1e20) saturates to MaxInt64 and was still sent. pickNum now returns an error and rejects a non-integral value, a NaN or Inf, a magnitude outside int32, and a non-numeric type. A missing key and an explicit null still report "not provided" without an error, so optional fields keep working. Every call site propagates the error, which means the whole --styles payload is refused before the document is created. Confirmed through the real CLI: font_size=12.9, col_sizes size=120.5 and size=1e20 all fail with a specific message while font_size=12 still passes. Tests: - TestPickNumRejectsNonIntegralAndOutOfRange covers eight rejected inputs plus five accepted ones, the missing key, an explicit null, and alias-key lookup. - Four new cases in TestSheetCreateValidatesBeforeCreatingDocument for cell_styles font_size, row_sizes size, col_sizes size and the overflow, each asserting calls == 0. - TestPickStrAndPickNum updated: a bool value now surfaces a type error with ok=true rather than being reported as absent. |
||
|
huwenjiao.hwj
|
0bb2b6ce98 |
feat(sheet): CSV export, style extensions and create-with-data
Adds four capabilities and, for the style surface, moves to the interface that can actually express them. Added: - sheet create --values / --sheets / --styles: create a workbook and populate it in one command. --values takes a 2D array into the default sheet, --sheets takes typed tables across several sheets, --styles carries cell_styles / row_sizes / col_sizes / cell_merges. Every structure and enum is validated before the document is created, so an invalid config never leaves an orphan empty document behind. - sheet export --export-format csv: synchronous single-sheet RFC4180 export with --sheet-id, --range and --value-render-option. --output writes to a file (a directory gets sheet-export.csv), otherwise the CSV goes to stdout while the truncation warning goes to stderr, keeping stdout pipeable. - sheet update-dimension --size-type: pixel / standard (restore the default row height or column width) / auto (fit row height to content, ROWS only). - sheet replace --match-formula: search and replace inside formula text. - sheet range set-style --font-style / --font-line / --font-family / --border-styles-json. - sheet range batch-set-style --ranges: stamp one style across several sheet-qualified ranges. Changed (breaking Schema change, no CLI break): - sheet range set-style moves from update_range to set_cell_range. The update_range style channel exposes exactly eight properties (backgroundColors, fontSizes, horizontalAlignments, verticalAlignments, fontColors, fontWeights, wordWrap, numberFormat) and has no slot for italic, underline/line-through, font family or borders, so the four new dimensions are not expressible there. interface_ref becomes set_cell_range and the twelve style flags stop publishing a flat property, because the value now lands in cells[i][j].cellStyles.* with no single top-level field to name. - sheet range batch-set-style submits one atomic batch_update instead of looping update_range, so a partial failure no longer leaves half the ranges stamped. --continue-on-error becomes a server passthrough. Caps the fan-out at 100 ranges and 200000 cells in aggregate. - sheet export and sheet create declare interface_mode=composite: both route across several tools depending on the flags, so a single mcp ref was wrong. Schema hygiene: - Nineteen parameters that previously resolved through flag_name_inference into property names present in no request (bgColor, exportFormat, values, ...) are now reviewed mapping exclusions with a stated reason, so Schema omits the property instead of inventing one. schema-compatibility reports 17 changed fields: 13 on sheet.range_set_style (interface_ref plus twelve property mappings) and 2 each on sheet.submit_export_job and sheet.create_workspace_sheet (interface_mode plus interface_ref). No CLI flag is removed and no command path changes; the same invocation runs on both the old and the new binary. Landing this needs a decision on the Schema contract break. Tests: targeted sheet suites in internal/helpers and the interface disposition contract tests in internal/app pass; make build and gofmt clean; check-schema-catalog, check-generated-drift, check-command-surface, check-skill-commands and check-runtime-confirmation-truth all pass. |