Repository navigation
导入前的列诊断与只读表头,命令行工具改为逐行读出 - #64
Conversation
标题差一个空格、多一个单位(「金额」与「金额(元)」),导入此前不报错,只是那一列悄悄全是默认值——OnCellError 只到单元格层面,管不到这里。新增 OnMissingColumn:读过表头之后、取第一行之前,每个对不上的标题上报一次,并给出表头上与之相近的标题(只按一方包含另一方判定,不多猜)。不设回调时行为与既有版本一致;在回调中抛出即可拒绝该文件。 新增 ReadHeader:只读到表头那一行为止,给出表名与各列标题,用于导入前核对列或据表头生成模型。 命令行工具随之改为逐行读出:表头单独读,数据走流式导入,类型推断改成边读边推断,写 JSON 也改为边读边写。二十万行五列:convert 13.5 秒 / 2412 MB → 2.6 秒 / 215 MB,--typed 12.9 秒 / 2428 MB → 4.1 秒 / 329 MB,generate-model 11.9 秒 / 2245 MB → 2.7 秒 / 204 MB,三者输出与此前逐字节相同。 顺带修掉一处:表头两侧有空白时,convert 写出的属性名保留空白而值永远为空——表头照原样取、数据却以去掉空白的标题为键,两边对不上。现在同出一处。 版本 2.12.0。 Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings remain for same-path truncation and formula handling, along with additional moderate and nit findings.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds v2.12.0 import diagnostics, header-only reading, and streaming CLI conversion.
Changes:
- Adds
OnMissingColumnandReadHeader. - Streams row reading, type inference, and JSON output.
- Updates documentation, tests, and version metadata.
File summaries
| File | Summary and final findings |
|---|---|
README.md |
Chinese release notes. |
README_EN.md |
English release notes. |
docs/versions/v2.12.0.md |
Release documentation. Nit (1 vote): examples are not directly runnable. Nit (1 vote): qualify shared-strings memory claims for .xlsx. |
docs/README.md |
Version index. |
Chsword.Excel2Object/Options/ExcelSheetHeader.cs |
Header result model. |
Chsword.Excel2Object/Options/ExcelImporterOptions.cs |
Missing-column callback. Nit (1 vote): XML example uses undeclared options and logger. |
Chsword.Excel2Object/Options/ExcelColumnMissing.cs |
Missing-column diagnostic model. |
Chsword.Excel2Object/Internal/XlsxRowReader.cs |
Worksheet source metadata. |
Chsword.Excel2Object/Internal/ImportRow.cs |
Worksheet row source model. |
Chsword.Excel2Object/Internal/ImportContext.cs |
Missing-column reporting. |
Chsword.Excel2Object/ExcelImporter.cs |
Header reading and model diagnostics. Moderate (3 votes): empty sheets bypass missing-column callbacks. Moderate (1 vote): full shared-strings loading affects header-read cost. Moderate (1 vote): malformed .xls files may be accepted as empty headers. Nit (1 vote): document stream-consumption differences. |
Chsword.Excel2Object/ExcelHelper.cs |
Exposes ReadHeader. |
Chsword.Excel2Object/Chsword.Excel2Object.csproj |
Version bump. |
Chsword.Excel2Object.Tests/ImportDiagnosticsTest.cs |
Diagnostic and header tests. Nit (1 vote): rename an ungrammatical test. Nit (3 votes): avoid the new null-forgiving operator. |
Chsword.Excel2Object.Cli/TypeInference.cs |
Incremental type inference. Moderate (2 votes): cache Result instead of allocating an array per cell. |
Chsword.Excel2Object.Cli/SheetData.cs |
Streaming sheet access. Critical (1 vote): formula cells may emit blanks because cached values are used. Moderate (1 vote): .xls inputs are reread multiple times. |
Chsword.Excel2Object.Cli/GenerateModelCommand.cs |
Uses incremental inference. |
Chsword.Excel2Object.Cli/ConvertCommand.cs |
Streaming JSON conversion. Moderate (1 vote): repeated per-cell inference result allocation. Critical (3 votes): same-path conversion truncates the source. Moderate (1 vote): stdout output still materializes the full JSON document. |
Review details
Suppressed comments (12)
Chsword.Excel2Object.Cli/ConvertCommand.cs:93
- For duplicate header titles, the old
JsonObjectassignment collapsed repeated keys to one property, matching the dictionary import's leftmost-column rule.Utf8JsonWriternow emits a property for every occurrence, so defaultconvertproduces duplicate JSON keys and is no longer byte-for-byte compatible for these sheets. Skip repeated titles while emitting each object.
writer.WritePropertyName(column);
Chsword.Excel2Object.Cli/ConvertCommand.cs:98
- The stateful
Inferenceobject still has aResultgetter that allocates and scans a candidate array on every call. Calling it inside the innermost cell loop repeats that work for every cell; for a large--typedexport this adds avoidable per-cell allocations, unlike the previous one-type-per-column map. Materialize each result once before writing rows or cache the result.
WriteTypedValue(writer, text, types[column].Result);
Chsword.Excel2Object.Cli/ConvertCommand.cs:73
- When stdout is used,
Runstill callsExcelToJson, which materializes the entire serialized document in aMemoryStreamand then a string beforeWriteLine. The default command therefore retains O(output-size) memory (and copies it), unlike the file-output path, so largeconvert input.xlsxinvocations do not get the claimed fully streaming behavior. Stream JSON directly to the suppliedTextWriter, or document this stdout limitation.
using var buffer = new MemoryStream();
WriteJson(path, sheet, typed, buffer);
return Encoding.UTF8.GetString(buffer.ToArray());
Chsword.Excel2Object.Cli/SheetData.cs:52
Columnsmay contain duplicate titles, and the importer deliberately resolves duplicates to the leftmost column (ExcelImporter.cs:179-185).ToDictionarynow throws for such a sheet, soconvert --typedandgenerate-modelregress instead of preserving the existing behavior. Initialize one inference per distinct title and reuse it for repeated columns.
var inferences = Columns.ToDictionary(c => c, _ => new TypeInference.Inference(), StringComparer.Ordinal);
Chsword.Excel2Object.Cli/SheetData.cs:36
- For
.xls,ReadHeadernecessarily reads the whole input, and eachRows()traversal invokes the whole-file fallback again. This makes defaultconvertandgenerate-modelread the file twice, andconvert --typedthree times; the previousSheetData.Loadread one byte array and reused it for its operations. Retain a whole-file representation for.xlsor avoid the extra full pass.
var header = ExcelHelper.ReadHeader(input, options => options.SheetTitle = sheetTitle);
Chsword.Excel2Object.Tests/ImportDiagnosticsTest.cs:51
- Rename this test to
AColumnMissingFromTheHeaderIsReported; the current name is ungrammatical and does not clearly state the behavior under test.
public void AColumnTheHeaderDoesNotHaveIsReported()
Chsword.Excel2Object/ExcelImporter.cs:106
- Although row XML reading stops at the header,
XlsxRowReader.IterateconstructsReadOnlySharedStringsTablebefore reading any row, and that table is built from the workbook's full shared-strings part. Data rows that introduce distinct strings therefore still affectReadHeadertime and memory, contrary to “后面有多少行数据都不影响开销”. Either avoid loading the full shared-strings table for header-only reads or qualify this contract.
/// 只读到表头那一行为止,后面有多少行数据都不影响其开销。<c>.xls</c> 仍须整份读入——该格式
/// 的数据并非顺序存放。传入的流由本方法读取,返回前即已读完。
Chsword.Excel2Object/ExcelImporter.cs:124
- For a seekable malformed or unsupported
.xls,GetDataRowscatches theWorkbookFactoryexception and returnsnull, and this branch turns it into an empty header. The CLI can therefore treat a bad.xlsas a valid zero-column sheet/[], whereas the previousSheetData.ReadHeaderthrew and.xlsxstill throws. Preserve the exception, or distinguish genuinely empty input, so invalid files are not silently accepted.
var source = LooksLikeXlsx(input)
? XlsxRowReader.Rows(input, options, context)
: GetDataRows(ReadAll(input), options, context);
if (source == null) return new ExcelSheetHeader(null, new List<string>());
Chsword.Excel2Object/ExcelImporter.cs:106
- The remarks say the input stream is fully consumed, but the XLSX path stops the row enumerator after the header and closes the package without reading the remaining rows. This contradicts the documented header-only behavior and can mislead callers about stream position and read cost; describe the XLSX and XLS cases separately.
/// 的数据并非顺序存放。传入的流由本方法读取,返回前即已读完。
Chsword.Excel2Object/Options/ExcelImporterOptions.cs:48
- The new public XML documentation example also relies on undeclared
optionsandlogger, so it cannot be compiled as written. Provide a complete minimal example (or make the snippet explicitly illustrative) to match the repository's runnable-example requirement.
/// <example>
/// <code>
/// // 只是记下来
/// options.OnMissingColumn = missing => logger.Warn(missing.ToString());
///
/// // 或者干脆不接受这样的文件
/// options.OnMissingColumn = missing => throw new Excel2ObjectException(missing.ToString());
docs/versions/v2.12.0.md:17
- This public API example is not directly runnable as required:
Order,bytes, andloggerare undeclared, andlogger.Warnhas no defined type or import. Replace it with a minimal complete example or include the necessary declarations.
```csharp
var orders = new ExcelImporter().ExcelToObject<Order>(bytes, options =>
options.OnMissingColumn = missing => logger.Warn(missing.ToString()));
docs/versions/v2.12.0.md:41
- For
.xlsx, the first row read constructsReadOnlySharedStringsTable, which contains strings from the whole workbook, including later data rows. Thus later rows with many distinct text values can still affectReadHeadermemory/time even though sheet rows are not traversed; qualify this statement to distinguish row traversal from shared-string loading.
只读到表头那一行为止,后面有多少行数据都不影响这一步的开销。用于在导入之前核对列、按表头生成模型,或让使用者自行把表里的列对到模型上。`SheetTitle` 是实际读的那张表的名字——未指定表名时即第一张表;`SheetTitle` 与 `TitleSkipLine` 两个选项照常生效。`.xls` 仍须整份读入,该格式的数据并非顺序存放。
- Files reviewed: 18/18 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| else | ||
| { | ||
| File.WriteAllText(destination, json); | ||
| using (var file = File.Create(destination)) WriteJson(input, sheet, typed, file); |
| foreach (var row in ExcelHelper.ExcelStreamToObject<Dictionary<string, object>>(input, | ||
| options => options.SheetTitle = _sheetTitle)) |
| foreach (var candidate in new[] | ||
| { | ||
| InferredType.Bool, InferredType.Int, InferredType.Long, InferredType.Decimal, | ||
| InferredType.DateTime | ||
| }) | ||
| if (_candidates.Contains(candidate)) | ||
| return candidate; |
| var dict = ExcelUtil.GetPropertiesAttributesDict<TModel>(); | ||
| var dictColumns = new Dictionary<int, KeyValuePair<PropertyInfo, ExcelTitleAttribute>>(); | ||
| var titleRow = result.Current; | ||
| if (titleRow == null) return dictColumns; |
| }); | ||
| Assert.IsNotNull(bytes); | ||
|
|
||
| using var input = new MemoryStream(bytes!); |
写出文件此前是先清空目标再边读边写:--output 指向输入本身时,源文件在读到之前就已被毁掉,中途失败也会留下半个文件。改为先写同目录下的临时文件,成功之后再就位。 CLI 改流式之后公式格取的是文件里存着的结果,而非当场求值——本库导出的文件没有存下结果,那一列会读作空白,与改动前不同。补 --whole:整份读入、公式当场求值,并在 README 与版本文档中写明默认行为。原先「输出逐字节相同」的说法改为「不含公式的文件逐字节相同」。 表里一行都没有时表头也没有,此前直接返回、模型上的每个标题都不上报;现同样上报。ReadHeader 遇到根本不是工作簿的输入改为抛出——此前静默给出空表头,CLI 因而把一份坏文件写成了 [],这是这次改动引入的回退,由新加的用例抓到。 另有 Inference.Result 每次取值都新建候选数组(逐格取,二十万行即上百万次),改为静态;Run 拆出目标路径的判定;测试里不再用 ! 压制可空警告。 Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
--whole 没登记进不带值的开关集合,紧随其后的位置参数会被当成它的值吃掉:excel2obj convert --whole a.xlsx 报「没有输入文件」。只有把它写在末尾才碰巧能用,而 README 与新加的用例恰好都那样写,因而没被发现。现补上登记,并把用例改为把它写在路径之前。 类型推断此前以标题为键收集,表头有重名的列时抛「同名的键已存在」。generate-model 本有 Unique 来给重名列起不同的属性名,这一改反倒让它在这种文件上挂掉。改为按列序号存放。 没有表名时,列缺失那句话会以一个孤零零的「的」开头。 Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
1. 模型上的列在表头中找不到时可以得知
标题差一个空格、多一个单位——「金额」与「金额(元)」——导入此前不会报错,只是那一列悄悄全是默认值。这是导入类问题里最常见的一种,而
OnCellError只覆盖到单元格层面,对此无能为力。回调在读过表头之后、取第一行之前调用,每个对不上的标题一次,与行数无关。相近的标题只按「一方包含另一方」判定,不作更多猜测。默认不设回调时行为与既有版本完全一致;在回调中抛出即可拒绝这样的文件。流式导入同样适用。
2. 只读出表头
只读到表头那一行为止,后面有多少行数据都不影响开销。用于导入前核对列、据表头生成模型,或让使用者自行把表里的列对到模型上。
SheetTitle与TitleSkipLine照常生效。3. 命令行工具改为逐行读出
excel2obj此前每条命令都把整个工作簿读进内存,且表头还要再打开一次文件。现在表头由ReadHeader单独读出、数据走流式导入,类型推断改成边读边推断(不再把整列的值攒起来),写 JSON 也改为边读边写(不再先搭出整棵 JSON 树)。二十万行五列,本机实测:
convertconvert --typedgenerate-model三条命令的输出与改动前逐字节相同(以
cmp校验)。--typed要先知道每列是什么类型,故读两遍文件。写的一侧(JSON → Excel)未改:那条路本来就要把整份 JSON 读进内存,且流式写入需要 SkiaSharp,而它并不随 NPOI 进入工具包。
4. 顺带修掉一处
表头两侧带空白时,
convert写出的属性名保留了空白,而它的值永远为空——表头是照原样取的,数据行却以去掉空白的标题为键,两边对不上:测试
新增 9 个用例:对不上的列上报一次并给出相近标题、全部对上时不报、不设回调时行为不变、回调抛出即拒绝、流式导入同样上报、多列缺失各报一次;
ReadHeader在两种格式下的表名与列、SheetTitle与TitleSkipLine、空表仍给出表名。全量 415 个用例在 net8.0 与 net10.0 通过,库在 7 个目标框架上 0 警告 0 错误。
版本 2.12.0,文档见
docs/versions/v2.12.0.md。🤖 Generated with Claude Code