refactor: remove dead code and duplication left by the resolution consolidation (#853) - #863
Merged
TomKaltofen merged 5 commits intoJul 20, 2026
Conversation
…ns (#853) - delete unread Engine.copy_compute_frameworks - make PARTIAL_RECORDS_CAP a single owner: FeatureResolutionError caps and snapshots the records, the engine passes them raw - collapse resolve_feature's winner framework-split to the seam's captured result.candidate_frameworks - move _as_str to components/utils.as_str, deduplicating the two copies
…le empty-environment render branch (#853) - remove ResolvedFeature.environment, which was always "standalone-default" - remove the empty-environment render branch and the now-dead environment_empty fact; PreFilterPlugins already owns the no-plugins-loaded message in production - migrate the tests and docs that pinned the removed surface
…rface (#853) The engine and resolve_feature resolve through evaluate(); the raising constructor, get(), .result and feature_group_compute_framework_mapping had no production caller and forced a cls.__new__ contortion inside evaluate(). - replace __init__ with a plain per-attempt memo initializer and build the instance in evaluate() with cls(...) instead of cls.__new__(cls) - drop get(), the .result attribute and feature_group_compute_framework_mapping - migrate the tests off the constructor to evaluate(), via a shared engine-seam helper (evaluate_or_raise / identify_winner) that mirrors how the engine renders and raises a resolution failure
FeatureGroups, ComputeFrameworks and Extenders each repeated the same "warn once per unregistered plugin" logic. Extract _warn_once_unregistered and call it from all three strict-mode filters.
#853) Restores positive coverage for the "No feature groups are loaded. Did you call PluginLoader.all()?" precondition, which PreFilterPlugins owns after the duplicate render-layer branch was deleted.
TomKaltofen
deleted the
chore/853-remove-dead-code-resolution-consolidation
branch
July 20, 2026 14:16
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.
Closes the #853 cleanup: removes dead surface and collapses duplicated logic left by the FeatureGroup-resolution consolidation. No production behavior change is intended.
Dead surface removed
Engine.copy_compute_frameworks(assigned, never read).IdentifyFeatureGroupClass(__init__that raised,get(),.result,feature_group_compute_framework_mapping). The engine andresolve_featurealready resolve throughevaluate(); removing the wrapper also drops thecls.__new__(cls)contortion insideevaluate()(now a plain__init__builds the per-attempt instance).environment_emptyfact.PreFilterPluginsraisesEnvironmentPreconditionError("No feature groups are loaded. Did you call PluginLoader.all()?")before an empty mapping can reach the renderer, so the render-layer duplicate was unreachable in production.ResolvedFeature.environment(always"standalone-default").Duplications collapsed to a single owner
_as_strmoved next tosafe_fieldascomponents/utils.as_str(was defined twice).PARTIAL_RECORDS_CAPis now applied in one place:FeatureResolutionError.__init__caps and snapshots (deep-copies) the records; the engine passes them raw.resolve_featurereads the winner's supported/unsupported framework split from the seam's capturedresult.candidate_frameworksinstead of recomputing it._warn_once_unregisteredhelper.Tests
Migrated ~15 test files off the removed raising constructor to
IdentifyFeatureGroupClass.evaluate(...), via a shared engine-seam helpertests/test_core/test_prepare/identify_seam.py(evaluate_or_raise/identify_winner) that mirrors how the engine renders and raises a resolution failure. Added a positive assertion for the nothing-loadedPluginLoader.all()precondition (now owned byPreFilterPlugins).Breaking change
ResolvedFeature.environment(a public dataclass field added in #757) is removed. It was a constant, the four-positional-argument constructor is unchanged, and #853 states backwards compatibility is not required, but external consumers readingresult.environmentare affected.Validation
toxgreen: 7115 passed, 170 skipped,ruff format/ruff checkclean, license allowlist OK,mypy --strictno issues in 880 files, bandit OK. Two independent deep reviews (a fresh Claude Opus agent and codex) found no correctness regressions; the one actionable finding (the missing positive assertion above) is addressed in this PR.