refactor(shared): merge expression_names into expression_registry (drop the one over-split)#83
Merged
Merged
Conversation
…op over-split) An over-split audit of the whole package (docs/OVER_SPLIT_AUDIT_2026.md) found the codebase is healthy — exactly ONE genuine over-split. shared/expression_names.py was a 12-line behavior-free re-export: reserved_expression_names() just forwarded to expression_registry, and is_reserved_expression_name() was byte-identical to the registry's own. Both already live in shared/expression_registry.py. It was not a layer boundary (same package), not an import firewall (expression_registry is itself stdlib-only and guarded lightweight), and not a compatibility contract (its only test asserted value-equality with the registry, i.e. confirmed its own redundancy). Merging removes one import hop with zero target-file growth. - Delete shared/expression_names.py. - Repoint the two production importers (shared/input_normalization.py, shared/computation_inputs.py) to shared.expression_registry. - Update test_expression_registry.py: drop the deleted module from the import-lightness guard and the redundant value-equality assertion; the registry-level assertions remain. Also archives the over-split audit report. ruff clean; 83 registry/consumer/ constants-editor tests pass; the registry lightness guard still holds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe module shared/expression_names.py, a thin re-export of two helper functions, is removed. Its consumers (shared/computation_inputs.py, shared/input_normalization.py) and a test (tests/test_expression_registry.py) now import those functions from shared/expression_registry.py instead. A new audit doc documents the rationale. ChangesExpression registry consolidation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)Not applicable for this change. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
2 participants
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.
Drop the one genuine over-split: merge
expression_namesintoexpression_registryAn over-split audit of the whole package (
docs/OVER_SPLIT_AUDIT_2026.md,6 dimensions, MERGE recommendations 2-skeptic verified) found the codebase is
healthy — not over-split. Of 15 candidate splits, exactly one was a
genuine over-split worth consolidating; everything else is a legitimate layer
boundary / single-source-of-truth / compatibility contract (documented in the
report's "keep these" list).
The one merge:
shared/expression_names.pywas a 12-line behavior-freere-export —
reserved_expression_names()just forwarded toexpression_registry, andis_reserved_expression_name()was byte-identical tothe registry's own. Both already live in
shared/expression_registry.py.itself stdlib-only + guarded lightweight), not a compatibility contract (its
only test asserted value-equality with the registry — confirming its own
redundancy).
(
shared/input_normalization.py,shared/computation_inputs.py) toshared.expression_registry; updatetests/test_expression_registry.py(drop the deleted module from the lightness guard + the redundant assertion).
Test plan
ruffclean; 83 registry/consumer/constants-editor tests pass; theregistry import-lightness guard still holds.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Refactor
Tests