Avoid per-cell allocations in XmlReaderHelper and TryParseCellReference

[virzak]

Fixes #1027

IsStartElement and GetAttribute now loop over the namespaces instead of allocating a LINQ closure per call, and TryParseCellReference parses the row digits in place instead of allocating a substring. The new test covers its edge cases.

QueryExcelBenchmark on net10.0, 15 iterations x 3 launches:

Method	master	this PR	Allocated
MiniExcel QueryFirst	27.71 ms	23.69 ms	62.44 -> 25.81 MB
MiniExcel Query with Mapping	333.16 ms	293.10 ms	718.05 -> 266.35 MB
MiniExcel Query	-	-	685.45 -> 233.54 MB

MiniExcel Query timings are left out because they are dominated by the lock fixed in #1023.

Summary by CodeRabbit

• Bug Fixes
  • Improved spreadsheet cell-reference parsing for malformed, zero, and excessively large row numbers.
  • Preserved namespace-order matching when finding XML elements and attributes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: be07949f-4d2b-44bd-b52a-9373bd52cfea

📥 Commits

Reviewing files that changed from the base of the PR and between ee0cfdd and 8d1b349.

📒 Files selected for processing: 1

• src/MiniExcel.Core/Helpers/CellReferenceConverter.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Cell-reference parsing now processes row digits directly and rejects invalid, non-positive, or out-of-range rows. XML namespace lookup methods now use loops instead of LINQ. Tests cover valid and malformed cell-reference row values.

Changes: Per-cell allocation reductions

Layer / File(s)	Summary
Parse cell-reference rows without substrings src/MiniExcel.Core/Helpers/CellReferenceConverter.cs, tests/MiniExcel.OpenXml.Tests/Issues/MiniExcelGithubIssuesTests.cs	TryParseCellReference parses row digits directly and rejects invalid, non-positive, or out-of-range rows. Theory cases test valid and malformed references.
Iterate XML namespaces directly src/MiniExcel.OpenXml/Utils/XmlReaderHelper.cs	IsStartElement and GetAttribute iterate namespaces in order and return the first match, or false or null when no match exists.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 8d1b3

This change reduces allocations during worksheet reading while preserving row validation and namespace lookup behavior. The supplied evidence shows no actionable merge-blocking risk.

Security architecture risk: 🔵 Low · up to 8d1b3

The parser retains its worksheet limits, and namespace lookup retains its existing precedence. Failed row parses now return different output values, which can change malformed-workbook processing. Equivalent resource-intensive processing was already reachable, and no new authority or broader security exposure was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A workbook author can influence merge-expansion work when merged-cell filling is enabled. Resetting failed row outputs broadens which malformed references trigger expansion, but does not increase the maximum independently attackable area: full-sheet ranges and row-zero failures already reached equivalent expansion in the base. The inspected impact remains processing work and query-local dictionaries, not new service or data-store authority.

Trust Boundaries and Controls

• observed — Table-query endpoint validation respects the parser’s success result before consuming coordinates. The merge reader’s unchecked-result behavior predates this PR; it is not evidence that this PR introduced a new trust-boundary bypass.

Resilience and Maintainability Implications

• observed — Merge expansion has no cancellation check inside its nested coordinate loops. Repeated queries reconstruct the dictionaries and repeat the work rather than resuming shared or durable state. These failure-containment characteristics are unchanged by this PR.

Hardening Proposals

• proposed — As separate hardening of the pre-existing merge-processing path, reject failed endpoint parses before dictionary mutation and bound expansion work with cancellation checks. This would reduce reliance on unspecified failure outputs and improve containment of expensive workbook inputs.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		The PR implements the allocation changes in #1027. XmlReaderHelper.IsStartElement and GetAttribute use direct namespace loops. TryParseCellReference parses row digits without Substring. The pa…	Use an overflow-safe range check before multiplying and adding the next digit. Add a regression test such as A4294967297. The parser must return false for every row above MaxRowNumber.
Docstring Coverage		Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately identifies both main changes: reducing per-cell allocations in XmlReaderHelper and avoiding substring allocations in TryParseCellReference.
Out of Scope Changes check		The changed implementation is limited to the methods named in #1027, the related cell-reference parser, and focused parser tests. The benchmark results support the allocation objective. No unrelated c…

Full details: Linked Issues check

Explanation

The PR implements the allocation changes in #1027. XmlReaderHelper.IsStartElement and GetAttribute use direct namespace loops. TryParseCellReference parses row digits without Substring. The parser still performs row = row * 10 + digit before the range check. In unchecked arithmetic, A4294967297 wraps the row to 1 and returns true, although the row exceeds MaxRowNumber. The added test does not cover this wrapping case.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}