fix(eval): sheet-name resolution is case-insensitive (Excel parity) - #4
Open
rmdort wants to merge 1 commit into
Open
fix(eval): sheet-name resolution is case-insensitive (Excel parity)#4rmdort wants to merge 1 commit into
rmdort wants to merge 1 commit into
Conversation
Excel resolves sheet names case-insensitively — =data!A1 reaches a sheet named Data — but SheetRegistry was an exact-match HashMap, so a case-mismatched reference either errored at ingest (Sheet not found -> surfaced as #REF! downstream) or, on the plan/id_for paths, silently minted a phantom sheet with the variant spelling. The engine's own consumers already assume case-insensitive semantics (rnc's reverse-dependency index and rename-rewrite both fold case), so a case-variant orphan could loop forever: matched case-insensitively for reinstall, rejected case-sensitively on install. SheetRegistry gains a case-folded secondary index (Unicode to_lowercase, matching normalize_name_key's folding for named ranges/tables) probed only after an exact-spelling miss: - get_id/id_for resolve case-variants to the existing sheet instead of erroring/minting phantoms; because the ref then interns as SheetKey::Id, reconstruction yields the display spelling and every downstream display-name lookup (arrow store, canonical hashing) is correct with no further changes - exact-first keeps any pre-existing pair of sheets differing only by case resolving each to itself, byte-identical to before - remove/rename keep the folded index in sync by re-pointing a shared folded key at a surviving case-sibling; case-only renames (Data -> DATA) keep resolving throughout - rename's collision check stays exact-spelling — rejecting case-collisions is a follow-up gated on callers surfacing the error Tests: SheetRegistry unit tests (exact priority, phantom-free id_for, remove/rename index sync, case-only rename, Unicode folding) + an engine- level resolution test. Full formualizer-eval suite: 2417 pass; the 3 extended_coverage IMEXP/IMSIN/IMCOS failures are pre-existing last-ULP float formatting diffs on this platform (fail identically on the base commit).
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
Excel resolves sheet names case-insensitively (
=data!A1reaches a sheet namedData), butSheetRegistrywas an exact-match map: a case-mismatched reference errored at ingest (→#REF!downstream) or silently minted a phantom sheet on theplan.rs/id_forpaths. The engine's own consumers already assume case-insensitive semantics (rnc's reverse-dependency index and rename-rewrite both fold case), so a case-variant orphan could loop forever: matched case-insensitively for reinstall, rejected case-sensitively on install.Found while scoping the unknown-sheet-
#REF!fix for the Athena rnc stability battery (rowsncolumns/spreadsheet#375 is the sibling change).Change
SheetRegistrygains a case-folded secondary index (Unicodeto_lowercase, matchingnormalize_name_key's folding for named ranges/tables), probed only after an exact-spelling miss:get_id/id_forresolve case-variants to the existing sheet instead of erroring or minting phantoms; the ref then interns asSheetKey::Id, so reconstruction yields the display spelling and every downstream display-name lookup (arrow store, canonical hashing) is correct with no further editsremove/renamere-point a shared folded key at a surviving case-sibling; case-only renames (Data→DATA) keep resolving throughoutrename's collision check stays exact-spelling — rejecting case-collisions Excel-style is a follow-up gated on callers surfacing the error (rnc currently discards it)Tests
SheetRegistryunit tests: exact priority, phantom-freeid_for, remove/rename index sync, case-only rename, Unicode foldingtest_sheet_reference_resolution_is_case_insensitive(sibling of the unknown-sheet test)formualizer-evalsuite: 2417 pass; the 3extended_coverageIMEXP/IMSIN/IMCOS failures are pre-existing last-ULP float-formatting platform diffs (identical on the base commit)