[codex] fix Python SDK decorator metadata duplication#693
[codex] fix Python SDK decorator metadata duplication#693Rachit-Gandhi wants to merge 4 commits into
Conversation
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff67bf85c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2760072a1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6238d1cdd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 400bd8377d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif explicit_triggers and existing_accepts_webhook == "warn": | ||
| resolved_accepts_webhook = True | ||
| elif existing_accepts_webhook is not None: | ||
| resolved_accepts_webhook = existing_accepts_webhook |
There was a problem hiding this comment.
When trigger sugar is staged on a wrapper that already has the default _accepts_webhook = "warn" (for example @app.reasoner() outside @on_event(...) outside a plain module-level @reasoner()), pending is merged above but explicit_triggers stays empty. This branch then treats the inner default as authoritative and returns "warn"; before this refactor the same staged trigger made accepts_webhook true, so code-declared webhook triggers lose the documented opt-in metadata.
Useful? React with 👍 / 👎.
Performance
✓ No regressions detected |
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
santoshkumarradha
left a comment
santoshkumarradha
left a comment
There was a problem hiding this comment.
Reviewed the decorator metadata refactor and ran the targeted Python tests locally. The metadata dedup and accepts_webhook handling look consistent now.
3 participants
Summary
Fixes #652.
This removes duplicated Python SDK decorator metadata handling by introducing a shared helper for:
@on_event/@on_scheduletriggers=[...]mergingcode_originstampingaccepts_webhookresolutionAgent.reasoner, module-level@reasoner, and router/agent direct-registration handling now share the same small metadata path while leaving the existing endpoint execution behavior inAgentunchanged.Root Cause
Agent.reasoner, module-level@reasoner, andAgentRoutereach had their own partial implementation of decorator metadata handling. That caused drift:@app.reasoner(triggers=[...])did not stamp triggercode_origin, and explicitaccepts_webhook=Falsecould be treated as missing during registration metadata serialization.Validation
uv run --extra dev pytest tests/test_decorator_code_origin.py tests/test_accepts_webhook.py tests/test_router.py tests/test_reasoner_path_normalization.py -quv run --extra dev pytest -quv run --extra dev python -m compileall agentfield/decorator_metadata.py agentfield/decorators.py agentfield/agent.py agentfield/router.pygit diff --check