Fix get_parameter_source() during type conversion and eager callbacks#3484
Open
yurishevtsov wants to merge 1 commit into
Open
Fix get_parameter_source() during type conversion and eager callbacks#3484yurishevtsov wants to merge 1 commit into
yurishevtsov wants to merge 1 commit into
Conversation
Record parameter source on the context immediately after consume_value(), before process_value() runs. In 8.4.0, set_parameter_source() was deferred until after type conversion and flag-group arbitration (0f71fe7, pallets#3403), so get_parameter_source() returned None inside ParamType.convert() and eager callbacks (pallets#3458). When several options share one parameter name, the losing option still sets a provisional source before process_value(); restore the previous source if arbitration rejects it so the winner's origin is not replaced unintentionally.
yurishevtsov
commented
May 20, 2026
| # Record the source before processing so eager callbacks and type | ||
| # conversion can inspect it. Restored after arbitration if this | ||
| # option loses a feature-switch group. | ||
| ctx.set_parameter_source(self.name, source) |
Author
yurishevtsov
commented
May 20, 2026
| ctx.set_parameter_source(self.name, source) | ||
| elif existing_source is not None: | ||
| # Lost arbitration; restore the winning option's source. | ||
| ctx.set_parameter_source(self.name, existing_source) |
Author
There was a problem hiding this comment.
This option already wrote its provisional source above; if it loses arbitration, put the winner’s source back so _parameter_source matches ctx.param.
yurishevtsov
commented
May 20, 2026
| ) | ||
|
|
||
| if is_winner: | ||
| ctx.set_parameter_source(self.name, source) |
Author
There was a problem hiding this comment.
Winner already has the right source from the early write. Only params need updating here.
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
Click 8.4.0 moved
set_parameter_source()to afterprocess_value()when fixing flag-group arbitration (#3403, #3404). That broke a case wasn't meant to change: anything that callsget_parameter_source()during type conversion or in an eager callback sawNone, even thoughconsume_value()had already figured out where the value came from (#3458).This is affecting Flask: with Click 8.4.0,
FLASK_DEBUG=1in the environment was ignored because_set_debugruns as an eager callback and could no longer see that the--debugflag was only at its default (pallets/flask#6025).The fix is to record the source right after
consume_value()and beforeprocess_value(), matching 8.3.3 timing for callbacks and custom types. Flag-group arbitration is the same as today.There is a second, narrower issue on the same code path: recording the source early means a losing option in a feature-switch group briefly overwrites
_parameter_sourcebefore arbitration runs. I am solving it by restoring the previous source when an option loses the slot. That path is easy to miss in tests (sharedoptsmay give the loser the same source as the winner), so there is a dedicated case intest_bool_flag_group_parameter_source. More detail is in the comments aroundhandle_parse_result.fixes #3458
Tests added / updated
tests/test_defaults.py— issue repro (ParamType.convert), eager callback, Flask-style_set_debugguardtests/test_options.py—test_bool_flag_group_parameter_source(includes a loser-restore case withenvvar, not only CLI +default_map)