PR Custody

fix: Handle Claude SDK structured output wrapper and improve validation

@AgentWrapperchecks n/achecks…fix/claude-structured-output-wrapper → next1 files · +58 −6updated 4mo ago

▸Description· 8 comments

Summary

Fixes TypeError: QASummary agent returned str instead of AgentResponse where structured output validation fails due to Claude SDK wrapping responses in {"output": {...}}.

What Changed

1. Unwrap `{"output": {...}}` wrapper before validation (with edge case handling)

Claude SDK wraps structured outputs, but Pydantic models expect properties at root level
Added detection and unwrapping logic in claude.py:369-394
Edge case handled: If the Pydantic model itself expects a single "output" field at root, don't unwrap (lines 378-394)
- Example: class MyModel(BaseModel): output: str → {"output": "hello"} stays as-is
- Checks model_fields to determine if unwrapping should occur
Prevents validation errors like "root: should NOT have additional properties"

2. Track multiple StructuredOutput calls (per SDK guidelines)

Per Claude Agents SDK: must pick the LAST structured output
Added counter and logging to track occurrences (lines 284, 320-335)
Existing code already kept last by overwriting, now explicit with logging

3. Improved error handling

Enhanced error messages with raw structured_output dump for debugging
Validate parsed output is not a string when output_type specified
Better error messages showing response preview when parsing fails
Prevents silent string returns that cause TypeError downstream

Root Cause

Production logs showed valid structured output failing validation and falling back to regex parsing:

{"output": {"summary": {"changes": [...]}, "verification": {"test_cases": [...]}}}
Output does not match required schema: root: should NOT have additional properties

The SDK wraps the response in {"output": {...}}, but AgentResponse expects summary and verification at root level. Without unwrapping, validation fails and the agent falls back to regex parsing (which can return strings instead of proper models).

How to Test

Run qa_summary_agent with structured output. Check logs for:

✅ "Unwrapping structured output from {"output": {...}} wrapper" (when model doesn't expect output field)
✅ "Keeping structured output as-is - model expects 'output' field at root" (edge case)
✅ "Successfully validated structured output from SDK"
✅ "Received N structured output(s), using the last one"
❌ No "SDK structured_output not available, falling back to regex" warnings

Notes

No database migrations needed
Backwards compatible - unwrapping only happens when wrapper detected AND model doesn't expect it
Handles edge case where model legitimately expects {"output": ...} structure
All checks passed: make fmt && make chk

@AgentWrapper5mo ago

Fixed in latest commit (c27317fd). The original logic checked if the model expects an 'output' field and kept the wrapper, but this failed if the SDK wraps everything (including models with 'output' fields).

New approach tries validation both ways: unwrap first, and only keep wrapped if unwrapping breaks validation. This robustly handles all edge cases including the one you flagged where class SimpleOutput(BaseModel): output: str receives {"output": {"output": "value"}}.

The try/catch ensures we always pick the correct interpretation regardless of SDK wrapping behavior.

@AgentWrapper4mo ago

Simplified EventCollector Serialization

You were absolutely right - the original implementation was overly complex with manual type checking for each message/block type.

Changes in 3dbbfc9:

Removed _serialize_message() (47 lines)
Removed _serialize_block() (39 lines)
Removed specific message type imports (8 lines)
Enhanced _serialize_value() to handle objects via vars()

Result:

288 lines (down from 378, -24%)
Generic serialization works for all SDK message types automatically
Proper JSON structure preserved (uses vars() to extract object attributes)
Simpler, more maintainable, handles future SDK changes automatically

How it works now:

Pydantic models → .model_dump()
Regular objects → vars(obj) to get attributes as dict
Recursive serialization for nested objects/lists
Fallback to str() only for truly non-serializable types

@AgentWrapper

loading diff…

What Changed

1. Unwrap {"output": {...}} wrapper before validation (with edge case handling)

Claude SDK wraps structured outputs, but Pydantic models expect properties at root level

Added detection and unwrapping logic in claude.py:369-394

Edge case handled: If the Pydantic model itself expects a single "output" field at root, don't unwrap (lines 378-394)

Example: class MyModel(BaseModel): output: str → {"output": "hello"} stays as-is
Checks model_fields to determine if unwrapping should occur

Prevents validation errors like "root: should NOT have additional properties"

2. Track multiple StructuredOutput calls (per SDK guidelines)

Per Claude Agents SDK: must pick the LAST structured output

Added counter and logging to track occurrences (lines 284, 320-335)

Existing code already kept last by overwriting, now explicit with logging

3. Improved error handling

Enhanced error messages with raw structured_output dump for debugging

Validate parsed output is not a string when output_type specified

Better error messages showing response preview when parsing fails

Prevents silent string returns that cause TypeError downstream

Root Cause

Production logs showed valid structured output failing validation and falling back to regex parsing:

{"output": {"summary": {"changes": [...]}, "verification": {"test_cases": [...]}}} Output does not match required schema: root: should NOT have additional properties

How to Test

Run qa_summary_agent with structured output. Check logs for:

✅ "Unwrapping structured output from {"output": {...}} wrapper" (when model doesn't expect output field)

✅ "Keeping structured output as-is - model expects 'output' field at root" (edge case)

✅ "Successfully validated structured output from SDK"

✅ "Received N structured output(s), using the last one"

❌ No "SDK structured_output not available, falling back to regex" warnings

@AgentWrapper4mo ago

✅ Verified in Staging

Triggered lint workflow with our branch and verified event file was uploaded to S3.

Workflow Details:

Workflow ID: wlbl6akr
App: slack
Branch: feature/claude-agent-event-observability
Agent: linter_agent
Execution ID: 4eb104eb-afac-47af-b76d-2f0ec620a15d

Verification:

✅ Agent execution logged execution_id at start:

Agent linter_agent | trace: https://app.datadoghq.com/llm/traces/span/3819683646570538709 | execution_id: 4eb104eb-afac-47af-b76d-2f0ec620a15d

✅ Event file successfully uploaded to S3:

Uploaded 11 events to S3: linter_agent/slack/wlbl6akr/run_1/4eb104eb-afac-47af-b76d-2f0ec620a15d_events.jsonl

✅ No errors or warnings in logs related to event collection

S3 Path:

s3://integrator-artifacts-staging/linter_agent/slack/wlbl6akr/run_1/4eb104eb-afac-47af-b76d-2f0ec620a15d_events.jsonl

Event Count: 11 events captured

The event collection system is working correctly in staging! 🎉

Summary

What Changed

1. Unwrap {"output": {...}} wrapper before validation (with edge case handling)

2. Track multiple StructuredOutput calls (per SDK guidelines)

3. Improved error handling

Root Cause

How to Test

Notes

Summary

What Changed

1. Unwrap {"output": {...}} wrapper before validation (with edge case handling)

2. Track multiple StructuredOutput calls (per SDK guidelines)

3. Improved error handling

Root Cause

How to Test

Notes

1. Unwrap `{"output": {...}}` wrapper before validation (with edge case handling)

1. Unwrap `{"output": {...}}` wrapper before validation (with edge case handling)