[andreatnvidia]

Summary

Anthropic MCP tool turns were rebuilt from text and tool calls, losing thinking signatures and redacted data. Replay each original assistant content array unchanged, including thinking interleaved with parallel tool calls, to preserve the message history required by Anthropic.

Related issue

Fixes #1118

Changes

• Carry opaque, provider-tagged ProviderReplayData through shared assistant messages, MCP execution/refusal, parser corrections, and restart checkpoints.
• Pass replay envelopes separately from canonical message dictionaries through ChatCompletionRequest.message_replay_data. Validate message alignment at construction and keep replay metadata out of transport kwargs.
• Let the matching adapter validate and interpret its own replay payload. The Anthropic adapter preserves its original content array and ignores unmatched provider tags.
• Keep ChatMessage.to_dict() provider-neutral and free of replay data; preserve existing trace and OpenAI-compatible serialization.
• Cover exact ordering, empty thinking, signatures, redacted data, repeated/parallel tool calls, unfamiliar opaque payloads, unmatched tags, system-message offsets, and request metadata exclusion. Document the replay boundary in the model architecture.

Testing

• All three package suites via .venv/bin/pytest: 5,051 passed, 11 skipped. Invoked directly because the Make targets use uv run.
• 949 model and MCP tests, plus Ruff lint/format checks.
• Original offline reproduction returns OK, preserving the signature and redacted data.
• Offline end-to-end simulation rerun after the boundary revision: preview, create, resume, reload, and export for execution and refusal scenarios. Verified 16 generated rows, 48 provider requests, 48 real stdio MCP calls, and exact replay of all assistant turns.
• Live Anthropic API acceptance remains untested; provider responses were simulated locally.

Checklist

• Follows commit message conventions.
• Commits are signed off (DCO).
• Linked issue is open and labeled triaged.
• Self-review and Claude recheck completed with no confirmed bugs or meaningful test gaps.
• Model architecture documentation updated.

[github-actions]

Fern preview: https://nvidia-preview-pr-1123.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable new issues were found.

Summary

Preserves Anthropic assistant content unchanged across MCP tool turns, including signed thinking and redacted blocks.

• Anthropic tool turns keep their original assistant content.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Anthropic assistant response] --> B[ProviderReplayData]
    B --> C[ChatMessage]
    C --> D[MCP turns and retry checkpoints]
    D --> E[ChatCompletionRequest]
    E --> F[Anthropic adapter]
    F --> G[Original content blocks]
    C --> H[Trace serialization]
    H --> I[Replay data excluded]

Loading

_{Reviews (2) · Last reviewed commit: "refactor: isolate provider replay handli..." · Reviewed by Greptile}

[svcnvidia-nemo-ci]

No confirmed Critical or Warning findings.

Reviewed all nine changed files and affected provider, MCP, generation, and trace contracts. Native assistant blocks remain intact through tool execution, refusals, parser corrections, and restart checkpoints; default trace and OpenAI-compatible serialization remain unchanged. The new integration tests cover these paths for synchronous and asynchronous generation.

Validation was static only: no tests were executed during this review, and live Anthropic API acceptance remains unverified.

Note: formal approval is not available for this request, so this review is posted as a comment only.

Review generated by codex. This was a full review. Request a full review with /review reset=true.

[nabinchha]

Thanks for putting this together, @andreatnvidia!

Summary

The fix preserves Anthropic assistant blocks through tool execution and retries, and the regression coverage exercises the important paths. Before merging, I’d like us to revise how provider replay data crosses the shared conversation layer.

Findings

Warnings — Worth addressing

packages/data-designer-engine/src/data_designer/engine/models/utils.py:112 — Keep provider replay handling behind the client boundary

• What: anthropic_content now appears on both shared message types and passes through ChatMessage.as_assistant(), MCP response handling, and generation. ChatMessage.to_dict(provider_type=...) also makes the shared conversation type responsible for selecting provider-specific request data.
• Why: This couples otherwise shared conversation and tool orchestration code to Anthropic’s wire representation. Supporting another provider with opaque replay requirements would extend the same provider-specific surface across these layers. Preserving signed blocks is necessary, but interpreting and serializing them should remain the adapter’s responsibility.
• Suggestion: Carry opaque, provider-tagged replay data on shared messages. Let the model facade and MCP preserve it without inspecting the payload, and let the matching adapter interpret it when constructing its request. Keep default trace serialization free of replay data. Please also revise the request boundary so the adapter can access replay data without adding provider selection to ChatMessage.to_dict(); renaming the field alone would leave that coupling in place. Retain the existing replay regression tests and verify that unmatched provider data is ignored and unfamiliar payloads pass through orchestration unchanged.

One possible shape for the shared envelope is:

@dataclass(frozen=True)
class ProviderReplayData:
    provider_type: str
    payload: object

Both AssistantMessage and ChatMessage could carry replay_data: ProviderReplayData | None, and ChatMessage.as_assistant() would accept that generic field. The Anthropic adapter would create the envelope from the original response blocks and consume it only when its provider tag matches. The payload stays opaque to the facade and MCP; the adapter owns any payload checks and interpretation.

This envelope is only part of the change: the client request boundary also needs to preserve access to replay data until the adapter builds the wire payload. Default ChatMessage.to_dict() should remain the canonical trace representation, without a provider-selection argument or replay data.

What Looks Good

• Replaying the original assistant content array preserves signatures, redacted thinking, whitespace, and interleaved ordering.
• Regression coverage includes synchronous and asynchronous execution, turn-limit refusals, corrections, and restart checkpoints.
• Default trace serialization excludes native replay data.

Residual Risk

At commit 3e8f398e, 310 focused tests passed, along with Ruff lint and formatting for all nine changed files. Live Anthropic API acceptance remains unverified; responses were simulated. The requested design revision will need those checks rerun.

Verdict

Needs changes — preserve the behavior and regression coverage, but move provider-specific replay interpretation and request serialization behind the adapter boundary before merge.

---

This review was generated by an AI assistant.

[andreatnvidia]

Thanks for the feedback, @nabinchha!

Updated in da79496c. Shared messages now carry opaque, provider-tagged ProviderReplayData, passed separately from canonical message dictionaries. Only the matching adapter validates and interprets the payload, and ChatMessage.to_dict() is provider-neutral.

Retained the replay regression tests and added coverage for unmatched provider tags and unfamiliar payloads passing through orchestration unchanged.

[nabinchha]

Thanks for putting this together, @andreatnvidia!

Summary

Preserves Anthropic assistant content across tool turns, corrections, and restarts. The latest revision addresses the earlier provider-boundary request: shared layers carry opaque tagged replay data, and the matching adapter owns payload interpretation.

Findings

No new actionable findings.

What Looks Good

• Opaque replay data remains separate from canonical message dictionaries and default trace serialization.
• The adapter preserves original block order, signatures, redacted content, and whitespace.
• Tests cover sync/async execution, refusals, retries, unfamiliar payloads, unmatched tags, and alignment checks.

Residual Risk

949 model and MCP tests passed, along with Ruff lint and formatting. Live Anthropic API acceptance remains unverified; HTTP responses were simulated.

Verdict

Ship it

---

This review was generated by an AI assistant.