refactor(context): remove unused provider formatters - #385
Conversation
AnthropicProviderFormatter and ResponsesProviderFormatter were not on the live path. RenderConfig defaults to OpenAIProviderFormatter for every client (Anthropic included; LiteLLM does the wire translation), and ResponsesProviderFormatter was an empty subclass whose only purpose was to be picked by an isinstance(llm_client, ResponsesClient) check in the runtime. ResponsesClient already projects the public message list itself. Delete both classes, the runtime's client-type dispatch, and Connect's api_style branch that selected between them. The runtime now uses RenderConfig.provider_formatter as configured for every client, so a wrapper around a UnifiedLLM client (for example an admission-control decorator) no longer has to be an instance of the wrapped class. Tests that only exercised the Anthropic export are removed; the rest move to OpenAIProviderFormatter with unchanged assertions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Paul Furgale <pfurgale@nvidia.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA-NeMo/labs-OO-Agents/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (20)
💤 Files with no reviewable changes (3)
Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change removes the Anthropic and Responses provider formatters. Runtime rendering now uses the configured formatter, and Connect setup uses ChangesProvider formatter consolidation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to The formatter consolidation is mergeable after normal checks; no concrete behavior regression remains identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 17.02% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 47 functions across 16 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
16baec6 to
3ed17c4
Compare
Summary
AnthropicProviderFormatterandResponsesProviderFormatterwere not on the live path, so this removes them along with the runtime'sisinstance(llm_client, ResponsesClient)dispatch that existed only to select the Responses one.RenderConfig.provider_formatterdefaults toOpenAIProviderFormatterfor every client. Anthropic models already go through it; LiteLLM does the wire translation. The Anthropic formatter's only callers were tests.ResponsesProviderFormatterwas an empty subclass of the OpenAI formatter.actor.pypicked it by client type, thenResponsesClient._transform_messagesprojected the same public message list it would have received anyway. Deleting it changes nothing on the wire.actor.pynow passesrender_config.provider_formatterstraight through. Connect's setup probe no longer branches onapi_styleto choose a formatter.OpenAIProviderFormatterstays: it converts the neutralRenderedMessagelist into the OpenAI-shapedlist[dict]that UnifiedLLM accepts, which is a step LiteLLM cannot do for us. Collapsing it to a plainto_messages()and droppingprovider_formatter=fromRenderConfig/render_contextis the remaining cleanup described in #337's migration notes; it overlaps #353's renderer rewrite, so it is left for after #353 lands.Why now
This is the one place the runtime cared about the concrete
UnifiedLLMsubclass. With it gone, a wrapper around a client (for example an admission-control decorator, see the discussion on #349) only needsacall,model, andcontext_windowto be a drop-inllm=for an agent.Changes
src/nooa/context_blocks/formatter.py: delete both classes and the Anthropic-only_arguments_objecthelper; module docstring updated.src/nooa/context_blocks/__init__.py: drop the two exports (public API removal).src/nooa/runtime/actor.py: delete_resolve_provider_formatter.src/nooa/unifiedllm/connect/_session.py: single formatter.OpenAIProviderFormatterwith unchanged assertions; helpers that took aresponses=flag to pick between identical formatters lose the flag. Oneparametrize("responses", ...)intest_response_history.pyis dropped for the same reason.CHANGELOG.mdentry under Unreleased.Test plan
uv run pytest --ignore=tests/integration tests: 8297 passed, 4 skipped, 83 deselectedtests/integration/test_truncation_e2e.py,test_l4_eviction_e2e.py,test_cache_resume_live.pycollected and passed/skipped as before (live cases skip without credentials)ruff check/ruff format --checkclean on all touched files🤖 Generated with Claude Code
Summary by CodeRabbit