feat(weave): Add sync/async generator support #3879
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=f1672aff294fdd7bd8962a90dfd038b20ae56a82 |
WalkthroughThis pull request refactors the accumulator functionality by renaming the public function Changes
Sequence Diagram(s)sequenceDiagram
participant Client as User/Application
participant Decorator as op Decorator
participant Tracer as Tracing Functions
participant Acc as _add_accumulator
Client->>Decorator: Call operation
Decorator->>Tracer: Check tracing conditions (_should_skip_tracing/_should_sample_traces)
Tracer-->>Decorator: Return operation result
Decorator->>Acc: Invoke accumulator wrapping
Acc-->>Decorator: Return accumulated result
Decorator-->>Client: Return final processed output
sequenceDiagram
participant Wrapper as Integration SDK Wrapper
participant Acc as _add_accumulator
participant Operation as Underlying Operation
Wrapper->>Acc: Call _add_accumulator(op, lambda)
Acc->>Operation: Execute operation
Operation-->>Acc: Return result
Acc-->>Wrapper: Return accumulated result
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (18)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)`**/*.py`: Focus on pythonic code patterns. Check for proper...
⏰ Context from checks skipped due to timeout of 90000ms (106)
🔇 Additional comments (56)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
weave/trace/op.py (1)
915-1001: Op decorator wrapper.
Though effective, you may consider a refactor to reduce the duplicated logic between sync/async generator creation.
🛑 Comments failed to post (1)
weave/trace/op.py (1)
311-319:
⚠️ Potential issuePotential name/logic inconsistency in
_should_sample_traces.
Currently, returningTruecauses tracing to be disabled viatracing_disabled(). The name suggests the opposite. Consider inverting the condition or renaming the function to avoid confusion.Possible fix:
- if random.random() > op.tracing_sample_rate: - return True # Sample traces for this call - return False + if random.random() <= op.tracing_sample_rate: + return True # Sample the tracing + return False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def _should_sample_traces(op: Op) -> bool: if call_context.get_current_call(): return False # Don't sample traces for child calls if random.random() <= op.tracing_sample_rate: return True # Sample the tracing return False
andrewtruong
force-pushed
the
andrew/lifecycle0b
branch
from
8caeaad to
f7fd755
Compare
andrewtruong
force-pushed
the
andrew/lifecycle0a
branch
2 times, most recently
from
5cc880c to
93b7c4c
Compare
andrewtruong
force-pushed
the
andrew/lifecycle0a
branch
from
3e3481a to
22918dc
Compare
andrewtruong
force-pushed
the
andrew/lifecycle0b
branch
from
281daed to
e8b5fb5
Compare
andrewtruong
force-pushed
the
andrew/lifecycle0b
branch
from
e8b5fb5 to
95f3208
Compare
Adds basic sync and async generator tracing support
See example for code:
Summary by CodeRabbit
Refactor
Tests