Skip to content

[codex] fix Python SDK decorator metadata duplication#693

Open
Rachit-Gandhi wants to merge 7 commits into
Agent-Field:mainfrom
Rachit-Gandhi:codex/issue-652-python-sdk-decorator-dedup
Open

[codex] fix Python SDK decorator metadata duplication#693
Rachit-Gandhi wants to merge 7 commits into
Agent-Field:mainfrom
Rachit-Gandhi:codex/issue-652-python-sdk-decorator-dedup

Conversation

@Rachit-Gandhi

Copy link
Copy Markdown

Summary

Fixes #652.

This removes duplicated Python SDK decorator metadata handling by introducing a shared helper for:

  • staged trigger collection from @on_event / @on_schedule
  • explicit triggers=[...] merging
  • code_origin stamping
  • accepts_webhook resolution
  • bare-decorator direct registration detection

Agent.reasoner, module-level @reasoner, and router/agent direct-registration handling now share the same small metadata path while leaving the existing endpoint execution behavior in Agent unchanged.

Root Cause

Agent.reasoner, module-level @reasoner, and AgentRouter each had their own partial implementation of decorator metadata handling. That caused drift: @app.reasoner(triggers=[...]) did not stamp trigger code_origin, and explicit accepts_webhook=False could 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 -q
  • uv run --extra dev pytest -q
  • uv run --extra dev python -m compileall agentfield/decorator_metadata.py agentfield/decorators.py agentfield/agent.py agentfield/router.py
  • git diff --check

@CLAassistant

CLAassistant commented Jun 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Rachit-Gandhi Rachit-Gandhi marked this pull request as ready for review June 27, 2026 05:23
@Rachit-Gandhi Rachit-Gandhi requested review from a team and AbirAbbas as code owners June 27, 2026 05:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread sdk/python/agentfield/agent.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread sdk/python/agentfield/decorator_metadata.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread sdk/python/agentfield/decorator_metadata.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread sdk/python/agentfield/decorator_metadata.py Outdated
@github-actions

github-actions Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Performance

SDK Memory Δ Latency Δ Tests Status
Python 9.4 KB +4% 0.30 µs -14%

✓ No regressions detected

@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 87.00% 87.40% ↓ -0.40 pp 🟡
sdk-go 91.80% 92.00% ↓ -0.20 pp 🟢
sdk-python 93.87% 93.73% ↑ +0.14 pp 🟢
sdk-typescript 90.05% 90.42% ↓ -0.37 pp 🟢
web-ui 84.82% 84.79% ↑ +0.03 pp 🟡
aggregate 85.62% 85.75% ↓ -0.13 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 0 ➖ no changes
sdk-go 0 ➖ no changes
sdk-python 0 ➖ no changes
sdk-typescript 0 ➖ no changes
web-ui 0 ➖ no changes

✅ Patch gate passed

Every surface whose lines were touched by this PR has patch coverage at or above the threshold.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the decorator metadata refactor and ran the targeted Python tests locally. The metadata dedup and accepts_webhook handling look consistent now.

auto-merge was automatically disabled June 28, 2026 05:36

Head branch was pushed to by a user without write access

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7e3506d82

ℹ️ 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".

Comment thread sdk/python/agentfield/agent.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7e3506d82

ℹ️ 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".

Comment thread sdk/python/agentfield/decorator_metadata.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 303bee4e02

ℹ️ 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".

vc_enabled=vc_setting,
triggers=list(decorator_triggers or []),
accepts_webhook=decorator_accepts_webhook,
triggers=list(merged_triggers or []),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep runtime trigger bindings visible to endpoints

When a plain @app.reasoner(triggers=[EventTrigger(..., transform=...)]) or router reasoner is registered, the merged triggers are now stored only in the ReasonerEntry; _execute_reasoner_endpoint still applies transforms by reading func._reasoner_triggers, and this refactor no longer stamps that attribute on the original function. In those webhook-triggered paths the trigger still registers, but the SDK skips the declared transform and invokes the reasoner with the raw event payload; either pass the entry bindings into execution or keep the runtime attribute in sync without reintroducing cross-agent leakage.

Useful? React with 👍 / 👎.

@santoshkumarradha santoshkumarradha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked the follow-up commits, merged main into the branch, and reran the Python SDK suite locally. The metadata handling stays correct across the stacked decorator cases, and the agent-local registration fix closes the leakage edge cleanly.

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.

[Python SDK] reasoner and skill decorators duplication

3 participants