[sdk/python] Preserve events when optional promoted fields are unset by SiddarthAA · Pull Request #791 · FailproofAI/failproofai

SiddarthAA · 2026-09-09T10:48:59Z

Summary

Omit promoted fields such as error_type and duration_ms when passed as None through **fields.
Warn about the omitted value instead of rejecting and losing the entire event.
Preserve strict validation for invalid non-null values.
Add regression coverage for string and numeric promoted fields, including successful agent_end events.

Why

Successful agent runs commonly have no error_type. The SDK previously serialized that value as null, rejected it during promoted-field validation, and silently lost the complete agent_end event in best-effort integrations.

Validation

998 passed, 6 skipped in sdk/python

Summary by CodeRabbit

Bug Fixes
- Fixed telemetry events with optional promoted fields set to None.
- Such fields are now omitted with a warning instead of being sent as explicit null values.
- Events such as successful agent_end calls are now recorded correctly without an unnecessary error field.
- Valid string and numeric values continue to be preserved.

Hermes review

Field	Value
Status	Approved
Reviewed commit	`7503d88b4f190e64aa20f56cd8f4b371b75a5a26`
Policy revision	`1d8f31d926828f3bae215c58f5b35baa44acbff0`
Model	`gpt-5.6-terra`
Duration	139s
Updated	2026-09-09T10:54:09.266006623+00:00

Summary

The promoted-field omission behavior works for ordinary custom fields, but duration_ms=None remains rejected on all auto-computed closing-event APIs, contrary to the stated behavior.

Changes

Drops None promoted custom fields with a warning before validation.
Adds regression coverage for promoted string and numeric fields.
Documents the Python SDK behavior in the changelog.

Validation

Passed docker run --rm --pull=always --network none --read-only --tmpfs /tmp:rw,noexec,nosuid,size=64m -e PYTHONDONTWRITEBYTECODE=1 -v "$PWD/sdk/python:/src:ro" -w /src python:3.11-slim python -c '<targeted promoted-field assertions>' — Confirmed promoted None custom fields are omitted, invalid non-null string/numeric values still raise, and agent_end(error_type=None) is emitted with a warning. (8s)
Passed docker run --rm --network none --read-only --tmpfs /tmp:rw,noexec,nosuid,size=64m -e PYTHONDONTWRITEBYTECODE=1 -v "$PWD/sdk/python:/src:ro" -w /src python:3.11-slim python -c '<auto-computed duration assertions>' — Confirmed the auto-computed-duration guards reject duration_ms=None on all four closing-event APIs, reproducing the finding. (0s)

Findings

No blocking findings.

1 advisory finding

Medium/High duration_ms=None is still rejected on closing events — tool_result, hook_completed, human_input, and agent_resume reject any duration_ms key before calling _validate_fields, so their duration_ms=None extras never reach the new omission-and-warning loop. An isolated container reproduced the rejection for all four methods. The added numeric test only invokes agent_start, where duration is not auto-computed. (sdk/python/failproofai_sdk/_events.py:393)

Open questions

None.

Policy overrides

None.

…he event A promoted key left at None in **fields reached the wire as an explicit JSON null, so _validate_promoted_string refused it. But None is how a caller says "I have no value", and the refusal landed inside their emit helper — which swallows telemetry errors, because telemetry must not break a run. The event vanished with nothing logged. agent_end(error_type=None) is the shape every SUCCESSFUL run produces: error_type is populated only on a failing outcome. Found against a real multi-agent app, where it dropped agent_end for every session that succeeded, leaving a dangling agent_start, no outcome, and no evaluation — the server triggers evaluation on agent_end. The same fix closes the mirror bug on promoted numerics. _build omits None only from a dataclass's named `specifics`; `extra` is merged verbatim. So duration_ms=None was dropped as a named parameter and written as an explicit null through **fields — same value, two outcomes, decided by which door it came through. Both paths now agree: for a promoted column, no value means no key. Dropped with a warning rather than silently: passing None is still a mistake worth hearing about, it just must not cost the event. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-09T10:49:08Z

Thanks @SiddarthAA for your contribution to Failproof AI! 🙌

We'd love to discuss your PR and welcome you to our community.

Discord: https://discord.befailproof.ai/
Reddit: https://www.reddit.com/r/failproofai/

coderabbitai · 2026-09-09T10:49:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4b604780-82c7-4982-93dd-003e03298eb7

📥 Commits

Reviewing files that changed from the base of the PR and between 7b88046 and 7503d88.

📒 Files selected for processing (3)

sdk/python/CHANGELOG.md
sdk/python/failproofai_sdk/_events.py
sdk/python/tests/test_server_contract.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

The Python SDK now removes None values from promoted numeric and string fields, logs warnings, and continues emitting events. Contract tests cover omission, warnings, successful agent_end events, and preserved non-None values. The changelog documents the behavior.

Changes

Promoted None handling

Layer / File(s)	Summary
Promoted field validation `sdk/python/failproofai_sdk/_events.py`	`_validate_fields` removes and warns on `None` values for promoted numeric and string keys. The validator documentation describes this behavior.
Event contract coverage `sdk/python/tests/test_server_contract.py`, `sdk/python/CHANGELOG.md`	Tests verify field omission, warning logs, successful `agent_end` emission, and preservation of real string values. The changelog documents the fix for promoted strings and numerics.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 7503d

The SDK now omits optional promoted fields passed as None while preserving event delivery and warning callers. Regression coverage confirms the intended event contract, with no actionable merge risk remaining.

Suggested reviewers: niveditjain

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 u…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving events when optional promoted fields are unset.
Description check	✅ Passed	The description explains the change, motivation, validation results, and regression coverage. It does not use the template headings or include the requested Type of Change and Checklist sections, but …

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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.

❤️ Share

_{A rabbit checks the fields with care
None hops out of every pair
Warnings softly mark the trail
Good events still cross the rail
Real values stay, precise and bright
The changelog records the flight}

_{Comment @coderabbitai help to get the list of available commands.}

hermes-exosphere · 2026-09-09T10:51:44Z

Hermes


Status	Reviewing
Verdict	Not reviewed yet
Head	`7503d88b4f19`
Rounds	0 of 5

No summary yet.

What this changes

No component map for this revision.

Rounds

No review has finished on this pull request yet.

Findings

Nothing raised yet.

@hermes-exosphere help lists every command. This comment is maintained in place — I rewrite it after each review rather than posting a new one.

hermes-exosphere · 2026-09-09T10:54:11Z

Hermes


Status	Reviewed
Verdict	Approved
Head	`7503d88b4f19`
Rounds	0 of 5

The promoted-field omission behavior works for ordinary custom fields, but duration_ms=None remains rejected on all auto-computed closing-event APIs, contrary to the stated behavior.

What this changes

flowchart LR
    n0PythontelemetryeventAPI["~ Python telemetry event API"]
    n1Eventpayloadserializer["Event payload serializer"]
    n2Servercontracttests["~ Server-contract tests"]
    n0PythontelemetryeventAPI -- "validated fields become payloads" --> n1Eventpayloadserializer
    n2Servercontracttests -- "exercises promoted fields" --> n0PythontelemetryeventAPI

Rounds

Round	Reviewed	Commits in this round	Verdict
0	`7503d88b4f19`	`7503d88b4f19`	Approved

Findings

Open

F1 duration_ms=None is still rejected on closing events (sdk/python/failproofai_sdk/_events.py) — round 1

@hermes-exosphere help lists every command. This comment is maintained in place — I rewrite it after each review rather than posting a new one.

hermes-exosphere

Hermes found no blocking issues in this revision.

1 advisory finding

Medium/High duration_ms=None is still rejected on closing events — tool_result, hook_completed, human_input, and agent_resume reject any duration_ms key before calling _validate_fields, so their duration_ms=None extras never reach the new omission-and-warning loop. An isolated container reproduced the rejection for all four methods. The added numeric test only invokes agent_start, where duration is not auto-computed. (sdk/python/failproofai_sdk/_events.py:393)

hermes-exosphere approved these changes Sep 9, 2026

View reviewed changes

SiddarthAA added the DO NOT MERGE label Sep 9, 2026

Navigation Menu

Uh oh!

[sdk/python] Preserve events when optional promoted fields are unset - #791

[sdk/python] Preserve events when optional promoted fields are unset#791
SiddarthAA wants to merge 1 commit into
mainfrom
feat/extend-daemon-sdk

SiddarthAA commented Sep 9, 2026 •

edited by hermes-exosphere

Loading

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

hermes-exosphere commented Sep 9, 2026

Uh oh!

hermes-exosphere commented Sep 9, 2026

Uh oh!

hermes-exosphere left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

SiddarthAA commented Sep 9, 2026 • edited by hermes-exosphere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Validation

Summary by CodeRabbit

Hermes review

Summary

Changes

Validation

Findings

Open questions

Policy overrides

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

coderabbitai Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

hermes-exosphere commented Sep 9, 2026

Hermes

What this changes

Rounds

Findings

Uh oh!

hermes-exosphere commented Sep 9, 2026

Hermes

What this changes

Rounds

Findings

Uh oh!

hermes-exosphere left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SiddarthAA commented Sep 9, 2026 •

edited by hermes-exosphere

Loading

coderabbitai Bot commented Sep 9, 2026 •

edited

Loading