Skip to content

Fix Optional[FromContext[T]] classification and define its semantics#235

Merged
timkpaine merged 2 commits into
nk/auto_deps_auto_callable_modelfrom
tkp/flow-model-optional-fromcontext
Jun 9, 2026
Merged

Fix Optional[FromContext[T]] classification and define its semantics#235
timkpaine merged 2 commits into
nk/auto_deps_auto_callable_modelfrom
tkp/flow-model-optional-fromcontext

Conversation

@timkpaine

Copy link
Copy Markdown
Member

_parse_annotation only peeled the top-level Annotated, so Optional[FromContext[int]] (a Union on the outside) was silently misclassified as a regular parameter and every call failed with a misleading "cannot satisfy unbound regular parameter" error.

Detect FromContext/Lazy markers nested inside a top-level Optional and define the two spellings precisely:

  • FromContext[Optional[int]]: contextual, required-in-context, value may be None.
  • Optional[FromContext[int]]: contextual, optional; absent -> bound to None (an implicit None default synthesized in _analyze_flow_function).

FromContext[Optional[int]] = None is therefore equivalent to Optional[FromContext[int]], and an explicit default still wins. Distinct required-ness yields distinct config/cache identities via the existing has_function_default/function_default identity terms.

Reject nested Lazy (Optional[Lazy[int]]) and non-Optional unions carrying FromContext (e.g. Union[FromContext[int], str]) with clear messages.

Adds focused regression tests covering all call shapes, the consistency equivalences, and the rejection cases.

`_parse_annotation` only peeled the top-level `Annotated`, so
`Optional[FromContext[int]]` (a Union on the outside) was silently
misclassified as a regular parameter and every call failed with a
misleading "cannot satisfy unbound regular parameter" error.

Detect `FromContext`/`Lazy` markers nested inside a top-level Optional and
define the two spellings precisely:

- `FromContext[Optional[int]]`: contextual, required-in-context, value may be None.
- `Optional[FromContext[int]]`: contextual, optional; absent -> bound to None
  (an implicit None default synthesized in `_analyze_flow_function`).

`FromContext[Optional[int]] = None` is therefore equivalent to
`Optional[FromContext[int]]`, and an explicit default still wins. Distinct
required-ness yields distinct config/cache identities via the existing
has_function_default/function_default identity terms.

Reject nested `Lazy` (`Optional[Lazy[int]]`) and non-Optional unions carrying
`FromContext` (e.g. `Union[FromContext[int], str]`) with clear messages.

Adds focused regression tests covering all call shapes, the consistency
equivalences, and the rejection cases.

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Test Results

    1 files  ± 0      1 suites  ±0   2m 50s ⏱️ -5s
1 129 tests  - 20  1 127 ✅  - 20  2 💤 ±0  0 ❌ ±0 
1 135 runs   - 20  1 133 ✅  - 20  2 💤 ±0  0 ❌ ±0 

Results for commit 2147313. ± Comparison against base commit d35c55d.

♻️ This comment has been updated with latest results.

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.64706% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (nk/auto_deps_auto_callable_model@d11a398). Learn more about missing BASE report.

Files with missing lines Patch % Lines
ccflow/tests/test_flow_model_optional_context.py 96.61% 2 Missing ⚠️
Additional details and impacted files
@@                         Coverage Diff                         @@
##             nk/auto_deps_auto_callable_model     #235   +/-   ##
===================================================================
  Coverage                                    ?   93.01%           
===================================================================
  Files                                       ?      160           
  Lines                                       ?    17835           
  Branches                                    ?     1153           
===================================================================
  Hits                                        ?    16589           
  Misses                                      ?     1020           
  Partials                                    ?      226           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@timkpaine timkpaine merged commit 51396bc into nk/auto_deps_auto_callable_model Jun 9, 2026
17 checks passed
@timkpaine timkpaine deleted the tkp/flow-model-optional-fromcontext branch June 9, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant