Refactored FluentMapping into separate assembly#991
Merged
michelebastione merged 3 commits intoJul 23, 2026
Merged
Conversation
…assembly This makes the main MiniExcel.OpenXml assembly smaller for the users who don't care for the fluent mapping
Manually added latest version of the package where NPOI brought in a vulnerable one
📝 WalkthroughWalkthroughChangesFluent OpenXML Mapping
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant MappingRegistry
participant MappingExporter
participant MappingWriter
participant OpenXmlWriter
Caller->>MappingRegistry: Configure<T>
MappingRegistry-->>Caller: compiled mapping
Caller->>MappingExporter: ExportAsync<T>
MappingExporter->>MappingWriter: SaveAsAsync
MappingWriter->>OpenXmlWriter: write mapped worksheet
OpenXmlWriter-->>Caller: export result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs (1)
54-55: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle cells without an
rattribute.Line 54 forces
withoutCrtofalse; whenaRis absent, the inherited parser retains the prior column index, so every cell in that row overwrites the sameMappedRowslot. Detect this mode asOpenXmlReaderdoes, or switch to sequential indexing when a cell reference is missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs` around lines 54 - 55, Update the cell-mapping logic in OpenXmlMappingReader to detect when a cell lacks its r attribute and enable the withoutCr/sequential-index path instead of always setting withoutCr to false. Match OpenXmlReader’s detection behavior or otherwise advance column indexes sequentially, while preserving referenced-cell handling.
🧹 Nitpick comments (1)
src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuffered template
MemoryStreamis never disposed.When
templateStreamisn't seekable it's copied intomemStream, which is reassigned totemplateStreambut never disposed. It's aMemoryStream(no unmanaged handles), so impact is minimal, but ausing/explicit dispose keeps lifetimes clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs` around lines 22 - 34, The non-seekable stream handling in MappingTemplateApplicator must dispose the buffered MemoryStream after template processing completes. Track the created stream separately or scope it with disposal while preserving templateStream usage throughout the operation, and ensure disposal occurs even when processing fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs`:
- Line 6: The collection mapping API must prevent an item mapping type from
disagreeing with the collection’s actual element type. Update
src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs#L6-L6
and
src/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cs#L9-L9
to expose or constrain an element-typed contract so unrelated types cannot be
selected; additionally update CollectionMappingBuilder configuration logic at
src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L43-L52
to inspect known generic collection element types and reject mismatches during
configuration.
In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs`:
- Around line 25-26: Update ToCell in
src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs
(lines 25-26) to validate parsed Excel coordinates against valid column and row
bounds, rejecting values such as A0, XFE1, and A1048577. Apply the same
bounds-aware validation to StartAt in
src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs
(lines 27-28), preferably through one shared validation helper.
In `@src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs`:
- Around line 13-19: Update ConversionHelper.ConvertValue to handle a null value
before calling value.GetType() or evaluating the assignability fast path. Return
the appropriate null result for nullable/reference targets while preserving the
existing conversion flow for non-null inputs.
In `@src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs`:
- Line 56: Validate the boolean result of TryParseCellReference for
prop.CellAddress in the mapping compilation flow, and surface a configuration
error when parsing fails instead of using default cellCol/cellRow values. Apply
the same validation to the corresponding TryParseCellReference call for
coll.StartCell, preserving normal mapping when both references parse
successfully.
---
Outside diff comments:
In `@src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs`:
- Around line 54-55: Update the cell-mapping logic in OpenXmlMappingReader to
detect when a cell lacks its r attribute and enable the
withoutCr/sequential-index path instead of always setting withoutCr to false.
Match OpenXmlReader’s detection behavior or otherwise advance column indexes
sequentially, while preserving referenced-cell handling.
---
Nitpick comments:
In `@src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs`:
- Around line 22-34: The non-seekable stream handling in
MappingTemplateApplicator must dispose the buffered MemoryStream after template
processing completes. Track the created stream separately or scope it with
disposal while preserving templateStream usage throughout the operation, and
ensure disposal occurs even when processing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4eb3bfd-e0e8-41fb-a589-002dd4414469
📒 Files selected for processing (32)
MiniExcel.slnxbenchmarks/MiniExcel.Benchmarks/MiniExcel.Benchmarks.csprojsrc/MiniExcel.OpenXml.FluentMapping/Api/MappingExporter.cssrc/MiniExcel.OpenXml.FluentMapping/Api/MappingImporter.cssrc/MiniExcel.OpenXml.FluentMapping/Api/MappingTemplater.cssrc/MiniExcel.OpenXml.FluentMapping/Api/ProviderExtensions.cssrc/MiniExcel.OpenXml.FluentMapping/CompiledMapping.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/IPropertyMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/MappingConfiguration.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/GlobalUsings.cssrc/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cssrc/MiniExcel.OpenXml.FluentMapping/Helpers/MappingMetadataExtractor.cssrc/MiniExcel.OpenXml.FluentMapping/MappedRow.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCellStream.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCellStreamAdapter.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cssrc/MiniExcel.OpenXml.FluentMapping/MappingReader.cssrc/MiniExcel.OpenXml.FluentMapping/MappingRegistry.cssrc/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cssrc/MiniExcel.OpenXml.FluentMapping/MappingTemplateProcessor.cssrc/MiniExcel.OpenXml.FluentMapping/MappingWriter.cssrc/MiniExcel.OpenXml.FluentMapping/MiniExcel.OpenXml.FluentMapping.csprojsrc/MiniExcel.OpenXml.FluentMapping/NestedMappingInfo.cssrc/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cssrc/MiniExcel.OpenXml/MiniExcel.OpenXml.csprojsrc/MiniExcel.OpenXml/Reader/OpenXmlReader.cssrc/MiniExcel/MiniExcel.csprojtests/MiniExcel.OpenXml.Tests/MiniExcel.OpenXml.Tests.csproj
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs (1)
54-55: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle cells without an
rattribute.Line 54 forces
withoutCrtofalse; whenaRis absent, the inherited parser retains the prior column index, so every cell in that row overwrites the sameMappedRowslot. Detect this mode asOpenXmlReaderdoes, or switch to sequential indexing when a cell reference is missing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs` around lines 54 - 55, Update the cell-mapping logic in OpenXmlMappingReader to detect when a cell lacks its r attribute and enable the withoutCr/sequential-index path instead of always setting withoutCr to false. Match OpenXmlReader’s detection behavior or otherwise advance column indexes sequentially, while preserving referenced-cell handling.
🧹 Nitpick comments (1)
src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs (1)
22-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuffered template
MemoryStreamis never disposed.When
templateStreamisn't seekable it's copied intomemStream, which is reassigned totemplateStreambut never disposed. It's aMemoryStream(no unmanaged handles), so impact is minimal, but ausing/explicit dispose keeps lifetimes clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs` around lines 22 - 34, The non-seekable stream handling in MappingTemplateApplicator must dispose the buffered MemoryStream after template processing completes. Track the created stream separately or scope it with disposal while preserving templateStream usage throughout the operation, and ensure disposal occurs even when processing fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs`:
- Line 6: The collection mapping API must prevent an item mapping type from
disagreeing with the collection’s actual element type. Update
src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs#L6-L6
and
src/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cs#L9-L9
to expose or constrain an element-typed contract so unrelated types cannot be
selected; additionally update CollectionMappingBuilder configuration logic at
src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L43-L52
to inspect known generic collection element types and reject mismatches during
configuration.
In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs`:
- Around line 25-26: Update ToCell in
src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs
(lines 25-26) to validate parsed Excel coordinates against valid column and row
bounds, rejecting values such as A0, XFE1, and A1048577. Apply the same
bounds-aware validation to StartAt in
src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs
(lines 27-28), preferably through one shared validation helper.
In `@src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs`:
- Around line 13-19: Update ConversionHelper.ConvertValue to handle a null value
before calling value.GetType() or evaluating the assignability fast path. Return
the appropriate null result for nullable/reference targets while preserving the
existing conversion flow for non-null inputs.
In `@src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs`:
- Line 56: Validate the boolean result of TryParseCellReference for
prop.CellAddress in the mapping compilation flow, and surface a configuration
error when parsing fails instead of using default cellCol/cellRow values. Apply
the same validation to the corresponding TryParseCellReference call for
coll.StartCell, preserving normal mapping when both references parse
successfully.
---
Outside diff comments:
In `@src/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cs`:
- Around line 54-55: Update the cell-mapping logic in OpenXmlMappingReader to
detect when a cell lacks its r attribute and enable the
withoutCr/sequential-index path instead of always setting withoutCr to false.
Match OpenXmlReader’s detection behavior or otherwise advance column indexes
sequentially, while preserving referenced-cell handling.
---
Nitpick comments:
In `@src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs`:
- Around line 22-34: The non-seekable stream handling in
MappingTemplateApplicator must dispose the buffered MemoryStream after template
processing completes. Track the created stream separately or scope it with
disposal while preserving templateStream usage throughout the operation, and
ensure disposal occurs even when processing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4eb3bfd-e0e8-41fb-a589-002dd4414469
📒 Files selected for processing (32)
MiniExcel.slnxbenchmarks/MiniExcel.Benchmarks/MiniExcel.Benchmarks.csprojsrc/MiniExcel.OpenXml.FluentMapping/Api/MappingExporter.cssrc/MiniExcel.OpenXml.FluentMapping/Api/MappingImporter.cssrc/MiniExcel.OpenXml.FluentMapping/Api/MappingTemplater.cssrc/MiniExcel.OpenXml.FluentMapping/Api/ProviderExtensions.cssrc/MiniExcel.OpenXml.FluentMapping/CompiledMapping.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/IPropertyMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/MappingConfiguration.cssrc/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cssrc/MiniExcel.OpenXml.FluentMapping/GlobalUsings.cssrc/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cssrc/MiniExcel.OpenXml.FluentMapping/Helpers/MappingMetadataExtractor.cssrc/MiniExcel.OpenXml.FluentMapping/MappedRow.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCellStream.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCellStreamAdapter.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cssrc/MiniExcel.OpenXml.FluentMapping/MappingReader.cssrc/MiniExcel.OpenXml.FluentMapping/MappingRegistry.cssrc/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cssrc/MiniExcel.OpenXml.FluentMapping/MappingTemplateProcessor.cssrc/MiniExcel.OpenXml.FluentMapping/MappingWriter.cssrc/MiniExcel.OpenXml.FluentMapping/MiniExcel.OpenXml.FluentMapping.csprojsrc/MiniExcel.OpenXml.FluentMapping/NestedMappingInfo.cssrc/MiniExcel.OpenXml.FluentMapping/OpenXmlMappingReader.cssrc/MiniExcel.OpenXml/MiniExcel.OpenXml.csprojsrc/MiniExcel.OpenXml/Reader/OpenXmlReader.cssrc/MiniExcel/MiniExcel.csprojtests/MiniExcel.OpenXml.Tests/MiniExcel.OpenXml.Tests.csproj
🛑 Comments failed to post (4)
src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs (1)
6-6: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve or validate the collection element type. The API permits mappings such as
Collection(x => x.Ints).WithItemMapping<Address>(...); the storedItemTypethen disagrees with the actual collection values and fails later during mapping execution.
src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs#L6-L6: expose an element-typed collection mapping contract where practical.src/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cs#L9-L9: prevent callers from selecting an unrelated item type.src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L43-L52: at minimum, inspect known generic collection element types and reject mismatches during configuration.📍 Affects 3 files
src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs#L6-L6(this comment)src/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cs#L9-L9src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L43-L52🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs` at line 6, The collection mapping API must prevent an item mapping type from disagreeing with the collection’s actual element type. Update src/MiniExcel.OpenXml.FluentMapping/Configuration/IMappingConfiguration.cs#L6-L6 and src/MiniExcel.OpenXml.FluentMapping/Configuration/ICollectionMappingBuilder.cs#L9-L9 to expose or constrain an element-typed contract so unrelated types cannot be selected; additionally update CollectionMappingBuilder configuration logic at src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L43-L52 to inspect known generic collection element types and reject mismatches during configuration.src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs (1)
25-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject out-of-range Excel coordinates. Both regexes accept invalid addresses such as
A0,XFE1, andA1048577, allowing invalid mappings to compile and fail or misaddress cells later.
src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs#L25-L26: validate row and column bounds inToCell.src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L27-L28: apply the same bounds-aware validation inStartAt, ideally via one shared helper.📍 Affects 2 files
src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs#L25-L26(this comment)src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs#L27-L28🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs` around lines 25 - 26, Update ToCell in src/MiniExcel.OpenXml.FluentMapping/Configuration/PropertyMappingBuilder.cs (lines 25-26) to validate parsed Excel coordinates against valid column and row bounds, rejecting values such as A0, XFE1, and A1048577. Apply the same bounds-aware validation to StartAt in src/MiniExcel.OpenXml.FluentMapping/Configuration/CollectionMappingBuilder.cs (lines 27-28), preferably through one shared validation helper.src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs (1)
13-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash rg -nP -C3 'CollectionItemConverter|ConvertValue\s*\(' src/MiniExcel.OpenXml.FluentMappingRepository: mini-software/MiniExcel
Length of output: 5831
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== MappingReader around converter call ==\n' sed -n '250,320p' src/MiniExcel.OpenXml.FluentMapping/MappingReader.cs printf '\n== MappingCompiler around precompiled converter ==\n' sed -n '730,760p' src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs printf '\n== ConversionHelper ==\n' sed -n '1,220p' src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs printf '\n== CompiledMapping CollectionItemConverter declaration ==\n' sed -n '250,275p' src/MiniExcel.OpenXml.FluentMapping/CompiledMapping.csRepository: mini-software/MiniExcel
Length of output: 14660
Guard
ConvertValueagainst null inputs.handler.CollectionItemConverteracceptsobject?, so a null cell can reachConversionHelper.ConvertValue;value.GetType()then throws before the fallback path runs. Add a null check before the fast path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs` around lines 13 - 19, Update ConversionHelper.ConvertValue to handle a null value before calling value.GetType() or evaluating the assignability fast path. Return the appropriate null result for nullable/reference targets while preserving the existing conversion flow for non-null inputs.src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs (1)
56-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
TryParseCellReferenceresult is ignored.If
prop.CellAddressfails to parse,cellCol/cellRowstay at their defaults (0/0) and the property is silently mapped to an invalid coordinate rather than surfacing a configuration error. Same pattern at Line 104 forcoll.StartCell.🛡️ Proposed guard
- CellReferenceConverter.TryParseCellReference(prop.CellAddress, out int cellCol, out int cellRow); + if (!CellReferenceConverter.TryParseCellReference(prop.CellAddress, out int cellCol, out int cellRow)) + throw new InvalidOperationException($"Invalid cell address '{prop.CellAddress}' for property '{propertyName}'.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (!CellReferenceConverter.TryParseCellReference(prop.CellAddress, out int cellCol, out int cellRow)) throw new InvalidOperationException($"Invalid cell address '{prop.CellAddress}' for property '{propertyName}'.");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs` at line 56, Validate the boolean result of TryParseCellReference for prop.CellAddress in the mapping compilation flow, and surface a configuration error when parsing fails instead of using default cellCol/cellRow values. Apply the same validation to the corresponding TryParseCellReference call for coll.StartCell, preserving normal mapping when both references parse successfully.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cs`:
- Around line 13-17: Update ConvertValue so a null input returns null only when
targetType supports null (nullable or reference types); for non-nullable value
types, preserve the existing default-value fallback via
Activator.CreateInstance. Add coverage verifying both nullable/reference targets
and non-nullable value targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09fff427-f573-4ac7-bba9-20c623589c0d
📒 Files selected for processing (3)
src/MiniExcel.OpenXml.FluentMapping/Helpers/ConversionHelper.cssrc/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cssrc/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/MiniExcel.OpenXml.FluentMapping/MappingTemplateApplicator.cs
- src/MiniExcel.OpenXml.FluentMapping/MappingCompiler.cs
michelebastione
force-pushed
the
fluent-mapping-assembly
branch
from
July 23, 2026 21:46
40ca879 to
2deb7e9
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.
This makes it easier to mantain and it also makes the main MiniExcel.OpenXml assembly smaller for the users who don't care for the fluent mapping.