Skip to content

Conversation

@NeejWeej
Copy link
Collaborator

No description provided.

@NeejWeej NeejWeej changed the base branch from main to nk/local_model_context_registration December 31, 2025 08:57
@github-actions
Copy link
Contributor

github-actions bot commented Dec 31, 2025

Test Results

566 tests  +21   562 ✅ +21   1m 42s ⏱️ +5s
  1 suites ± 0     4 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 95119e3. ± Comparison against base commit a56d526.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 94.95798% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.51%. Comparing base (a56d526) to head (95119e3).
⚠️ Report is 2 commits behind head on nk/local_model_context_registration.

Files with missing lines Patch % Lines
ccflow/tests/test_callable.py 92.70% 10 Missing ⚠️
ccflow/callable.py 96.87% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                          Coverage Diff                          @@
##           nk/local_model_context_registration     #170    +/-   ##
=====================================================================
  Coverage                                95.50%   95.51%            
=====================================================================
  Files                                      132      132            
  Lines                                     8463     8689   +226     
  Branches                                   521      534    +13     
=====================================================================
+ Hits                                      8083     8299   +216     
- Misses                                     262      272    +10     
  Partials                                   118      118            

☔ View full report in Codecov by Sentry.
📢 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.

@NeejWeej NeejWeej marked this pull request as ready for review December 31, 2025 09:37
@ptomecek
Copy link
Collaborator

From an implementation perspective, I get why it looks like this:

            @Flow.call
            @dynamic_context
            def __call__(self, *, x: int, y: str = "default") -> GenericResult:
                return GenericResult(value=f"{x}-{y}")

But I feel like from a user perspective, it would be nice to have it look like this

            @Flow.call(dynamic_context=True)
            def __call__(self, *, x: int, y: str = "default") -> GenericResult:
                return GenericResult(value=f"{x}-{y}")

@ptomecek
Copy link
Collaborator

I also wonder whether calling it generate_context would be better as the context itself isn't dynamic. We're just auto-generating one for that model specifically.

@timkpaine
Copy link
Member

why does the user have to indicate that its a dynamic context at all manually? isn't this implied by not having a single arg context of type inheriting from BaseContext?

@NeejWeej NeejWeej force-pushed the nk/add_dynamic_context_v2 branch from 1600618 to f89ff9f Compare December 31, 2025 21:02
@NeejWeej
Copy link
Collaborator Author

NeejWeej commented Jan 1, 2026

why does the user have to indicate that its a dynamic context at all manually? isn't this implied by not having a single arg context of type inheriting from BaseContext?

I'm a bit wary of this because it feels like it can open a whole can of worms of edge cases (generics, forward reference for contexts), and might be confusing if someone makes a mistake (maybe names context something like contex which would now auto generate the context). Plus, at least in the beginning, I feel like it's safer to make this feature opt-in.

@NeejWeej
Copy link
Collaborator Author

NeejWeej commented Jan 1, 2026

I also wonder whether calling it generate_context would be better as the context itself isn't dynamic. We're just auto-generating one for that model specifically.

good point, I went with auto_context (save a few key strokes) and allowed it to take a class to specify a parent class (to keep that functionality, but not add extra parameters and the possibility of a contradictory state, like auto_context=False but a parent context is specified)

# Auto Context Example:
class MyModel(CallableModel):
    @Flow.call(auto_context=True)
    def __call__(self, *, x: int, y: str = "default") -> GenericResult:
        return GenericResult(value=f"{x}-{y}")
model = MyModel()
model(x=42)  # Call with kwargs directly
# With Parent Context:
class MyModel2(CallableModel):
    @Flow.call(auto_context=DateContext)
    def __call__(self, *, date: date, extra: int = 0) -> GenericResult:
        return GenericResult(value=date.day + extra)
# The generated context inherits from DateContext, so it's compatible
# with infrastructure expecting DateContext instances.

@NeejWeej NeejWeej changed the title Nk/add dynamic context v2 Add ability to auto-generate context classes from function signature Jan 1, 2026
Base automatically changed from nk/local_model_context_registration to main January 7, 2026 20:06
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.

4 participants