Skip to content

refactor: codec-owned provider-surface detection via a built-in registry#301

Open
zhongxuanwang-nv wants to merge 1 commit into
NVIDIA:mainfrom
zhongxuanwang-nv:refactor/relay-362-codec-surface-registry
Open

refactor: codec-owned provider-surface detection via a built-in registry#301
zhongxuanwang-nv wants to merge 1 commit into
NVIDIA:mainfrom
zhongxuanwang-nv:refactor/relay-362-codec-surface-registry

Conversation

@zhongxuanwang-nv

@zhongxuanwang-nv zhongxuanwang-nv commented Jun 24, 2026

Copy link
Copy Markdown
Member

Overview

Implements RELAY-362, the follow-up requested in review of #291: move provider-surface detection out of the hard-coded if/else chains in codec::resolve into codec-owned descriptors iterated through a small built-in registry. resolve.rs is now provider-agnostic (no provider field-name literals), new built-in surfaces become additive, and a codec-owned provider hint can disambiguate the one ambiguous request shape (an Anthropic request without a top-level system). The change is additive, Rust-core-only, and behavior-preserving: every existing codec::resolve test passes unchanged and the adaptive path is untouched.

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • codec::resolve — added a pub(crate) SurfaceDescriptor (a hint-aware request detector, a response detector, and decode entry points) and a by-value static REGISTRY ordered by request-detection priority (Responses > Anthropic > Chat). The public functions are now provider-agnostic policy only: first registry match for requests, unique match for responses, fail-open normalize_*. ProviderSurface and every existing public signature are unchanged.
  • New pub fn detect_request_surface_with_hint(body, Option<&str>)detect_request_surface delegates to it with None for exact parity. A recognized hint can only upgrade the ambiguous messages-only shape; registry priority order keeps it from overriding a strong input/instructions/system signal.
  • Codec-owned detection — each built-in codec module (openai_chat, openai_responses, anthropic) owns its detectors (as closures) and decode entry points in a SURFACE_DESCRIPTOR const. Only Anthropic's request detector is hint-aware (it claims a system-less messages body when the hint is "anthropic"); the OpenAI detectors ignore the hint. Adding a future built-in surface (e.g. Gemini) means registering a descriptor, not editing detection logic.
  • Not wired into adaptive (deferred follow-up) — the hint is available but not yet called. Wiring it into resolve_request_surface / build_semantic_request_view would change ACG pass-through behavior, so it is intentionally left to a separate follow-up. The shape-only ACG path and its source-guard test are untouched.
  • Tests — added hint coverage (parity with a None hint, the Anthropic upgrade, "never overrides a strong signal", non-object input); the existing detection/normalization tests are unchanged.

No breaking changes. Local validation: cargo test -p nemo-relay, -p nemo-relay-adaptive, and -p nemo-relay-pii-redaction pass; cargo clippy -p nemo-relay --all-features --all-targets, cargo fmt --check, and cargo doc -p nemo-relay are clean for the changed files. The full cross-language matrix is left to CI (no binding surfaces codec::resolve).

Where should the reviewer start?

crates/core/src/codec/resolve.rs — the SurfaceDescriptor + REGISTRY and the detection-policy functions (detect_request_surface_with_hint, detect_response_surface). Then the per-codec SURFACE_DESCRIPTOR blocks, especially anthropic.rs's hint-aware detect_request: registry priority order (Responses before Anthropic) is what keeps the "anthropic" hint from overriding a strong input/instructions signal.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Closes RELAY-362

Summary by CodeRabbit

  • New Features

    • Improved request and response detection for supported chat/response formats, including better handling of Anthropic and OpenAI variants.
    • Added support for provider hints when identifying request formats, helping resolve ambiguous cases more accurately.
  • Bug Fixes

    • Reduced incorrect format classification by requiring clearer matches for responses.
    • Preserved existing behavior for unrecognized or non-object payloads.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: cafe0212-7b8f-4c14-b6cb-881f533b4aba

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The PR adds registry-backed surface descriptors for OpenAI Chat, OpenAI Responses, and Anthropic Messages, routes request/response normalization through the registry, and adds tests for hint-aware request detection.

Changes

Registry-backed codec surface detection

Layer / File(s) Summary
Registry and normalization flow
crates/core/src/codec/resolve.rs
SurfaceDescriptor and detector plumbing are added, request detection accepts an optional provider hint, response detection requires exactly one matching descriptor, and normalization decodes through the selected descriptor.
Built-in codec descriptors
crates/core/src/codec/openai_responses.rs, crates/core/src/codec/anthropic.rs, crates/core/src/codec/openai_chat.rs
OpenAI Responses, Anthropic Messages, and OpenAI Chat each add a SURFACE_DESCRIPTOR with JSON shape predicates and decode routing to the codec implementation.
Hint-aware detection tests
crates/core/tests/unit/codec/resolve_tests.rs
Unit tests cover detect_request_surface_with_hint across matching None, provider-specific hints, strong signals, and non-object or keyless bodies.

Estimated review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Title check ❌ Error The title matches the change but is exactly 72 characters, so it does not satisfy the under-72-character requirement. Shorten the title to fewer than 72 characters while keeping the Conventional Commits format.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description includes all required sections, checklist items, reviewer guidance, and a related issue reference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions github-actions Bot added size:M PR is medium Improvement improvement to existing functionality lang:rust PR changes/introduces Rust code labels Jun 24, 2026
@github-actions

Copy link
Copy Markdown

@zhongxuanwang-nv zhongxuanwang-nv self-assigned this Jun 25, 2026
@zhongxuanwang-nv zhongxuanwang-nv added this to the 0.5 milestone Jun 25, 2026
@zhongxuanwang-nv zhongxuanwang-nv added the DO NOT MERGE PR should not be merged; see PR for details label Jun 25, 2026
@zhongxuanwang-nv zhongxuanwang-nv force-pushed the refactor/relay-362-codec-surface-registry branch from 7060394 to a881241 Compare June 25, 2026 01:05
@zhongxuanwang-nv zhongxuanwang-nv force-pushed the refactor/relay-362-codec-surface-registry branch from a881241 to 005d547 Compare June 25, 2026 01:18
@zhongxuanwang-nv zhongxuanwang-nv force-pushed the refactor/relay-362-codec-surface-registry branch from 005d547 to 8d45d8d Compare June 25, 2026 02:43
Move provider-surface detection out of the hard-coded if/else chains in codec::resolve into codec-owned SurfaceDescriptors iterated through a small built-in registry. resolve.rs is now provider-agnostic (no field-name literals); each built-in codec owns its detection and decode logic in a SURFACE_DESCRIPTOR const.

Add detect_request_surface_with_hint(body, Option<&str>); detect_request_surface delegates to it with None for exact parity. A recognized provider hint can upgrade the ambiguous messages-only shape (the Anthropic detector claims it for provider "anthropic"), while registry priority order keeps it from overriding a strong input/instructions/system signal. The hint is not wired into adaptive, so behavior is preserved and all existing codec::resolve tests pass unchanged.

Relates to RELAY-362

Signed-off-by: Zhongxuan Wang <[email protected]>
@zhongxuanwang-nv zhongxuanwang-nv force-pushed the refactor/relay-362-codec-surface-registry branch from 8d45d8d to 261fc7e Compare June 25, 2026 02:48
@zhongxuanwang-nv zhongxuanwang-nv marked this pull request as ready for review June 25, 2026 02:49
@zhongxuanwang-nv zhongxuanwang-nv requested a review from a team as a code owner June 25, 2026 02:49
@zhongxuanwang-nv zhongxuanwang-nv removed the DO NOT MERGE PR should not be merged; see PR for details label Jun 25, 2026
@zhongxuanwang-nv

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/core/src/codec/resolve.rs`:
- Around line 82-106: The exact-one response descriptor selection rule is
duplicated between detect_response_surface and normalize_response, so extract
that matching logic into a single private helper and have both functions use it.
Keep the helper responsible for iterating REGISTRY and returning the sole
matching descriptor only when there is exactly one match, then reuse it in both
detect_response_surface and normalize_response so any future change to matching
behavior stays consistent.

In `@crates/core/tests/unit/codec/resolve_tests.rs`:
- Around line 269-289: Add a response-side unit test for the “exactly one match”
behavior in detect_response_surface/normalize_response: create cases where a
response body matches zero detectors and where it matches two or more detectors,
and assert the result is None. Place the test near
hint_never_overrides_strong_signals so the new contract is covered alongside the
existing resolve_tests helpers and detector logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 3bb781d9-a210-4683-bd6e-49bb1898f05d

📥 Commits

Reviewing files that changed from the base of the PR and between 6b1ce42 and 261fc7e.

📒 Files selected for processing (5)
  • crates/core/src/codec/anthropic.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Check / Run
  • GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (14)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

crates/core/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
crates/{core,adaptive}/**

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
crates/{core,adaptive}/**/*.rs

⚙️ CodeRabbit configuration file

crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.

Files:

  • crates/core/src/codec/openai_responses.rs
  • crates/core/src/codec/openai_chat.rs
  • crates/core/tests/unit/codec/resolve_tests.rs
  • crates/core/src/codec/resolve.rs
  • crates/core/src/codec/anthropic.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Maintain documented and tested validation and report behavior for adaptive surfaces

Files:

  • crates/core/tests/unit/codec/resolve_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}

⚙️ CodeRabbit configuration file

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.

Files:

  • crates/core/tests/unit/codec/resolve_tests.rs
🔇 Additional comments (6)
crates/core/src/codec/resolve.rs (2)

27-48: LGTM!


58-73: LGTM!

crates/core/src/codec/openai_responses.rs (1)

46-55: LGTM!

crates/core/src/codec/anthropic.rs (1)

47-60: LGTM!

crates/core/src/codec/openai_chat.rs (1)

34-40: LGTM!

crates/core/tests/unit/codec/resolve_tests.rs (1)

226-301: LGTM!

Comment on lines +82 to +106
let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
match (matches.next(), matches.next()) {
(Some(descriptor), None) => Some(descriptor.surface),
_ => None,
}
}

/// Best-effort decode of a raw request into [`AnnotatedLlmRequest`] (fail-open).
#[must_use]
pub fn normalize_request(request: &LlmRequest) -> Option<AnnotatedLlmRequest> {
match detect_request_surface(&request.content)? {
ProviderSurface::OpenAIChat => OpenAIChatCodec.decode(request),
ProviderSurface::OpenAIResponses => OpenAIResponsesCodec.decode(request),
ProviderSurface::AnthropicMessages => AnthropicMessagesCodec.decode(request),
}
.ok()
let obj = request.content.as_object()?;
let descriptor = REGISTRY.iter().find(|d| (d.detect_request)(obj, None))?;
(descriptor.decode_request)(request).ok()
}

/// Best-effort decode of a raw response into [`AnnotatedLlmResponse`] (fail-open).
#[must_use]
pub fn normalize_response(raw: &Json) -> Option<AnnotatedLlmResponse> {
match detect_response_surface(raw)? {
ProviderSurface::OpenAIChat => OpenAIChatCodec.decode_response(raw),
ProviderSurface::OpenAIResponses => OpenAIResponsesCodec.decode_response(raw),
ProviderSurface::AnthropicMessages => AnthropicMessagesCodec.decode_response(raw),
}
.ok()
let obj = raw.as_object()?;
let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
let descriptor = match (matches.next(), matches.next()) {
(Some(descriptor), None) => descriptor,
_ => return None,
};
(descriptor.decode_response)(raw).ok()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

De-duplicate the "exactly one matching response descriptor" rule.

detect_response_surface (Lines 82-87) and normalize_response (Lines 101-106) independently re-implement the same matches.next(), matches.next() exactly-one selection. If one is later changed (e.g., to first-match), detection and normalization will silently disagree on which surface a response belongs to. Extract a single private helper and have both call it.

♻️ Proposed helper to keep the rule in one place
+fn unique_response_descriptor(obj: &serde_json::Map<String, Json>) -> Option<&'static SurfaceDescriptor> {
+    let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
+    match (matches.next(), matches.next()) {
+        (Some(descriptor), None) => Some(descriptor),
+        _ => None,
+    }
+}
+
 pub fn detect_response_surface(raw: &Json) -> Option<ProviderSurface> {
     let obj = raw.as_object()?;
-    let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
-    match (matches.next(), matches.next()) {
-        (Some(descriptor), None) => Some(descriptor.surface),
-        _ => None,
-    }
+    unique_response_descriptor(obj).map(|d| d.surface)
 }

 #[must_use]
 pub fn normalize_response(raw: &Json) -> Option<AnnotatedLlmResponse> {
     let obj = raw.as_object()?;
-    let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
-    let descriptor = match (matches.next(), matches.next()) {
-        (Some(descriptor), None) => descriptor,
-        _ => return None,
-    };
+    let descriptor = unique_response_descriptor(obj)?;
     (descriptor.decode_response)(raw).ok()
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
match (matches.next(), matches.next()) {
(Some(descriptor), None) => Some(descriptor.surface),
_ => None,
}
}
/// Best-effort decode of a raw request into [`AnnotatedLlmRequest`] (fail-open).
#[must_use]
pub fn normalize_request(request: &LlmRequest) -> Option<AnnotatedLlmRequest> {
match detect_request_surface(&request.content)? {
ProviderSurface::OpenAIChat => OpenAIChatCodec.decode(request),
ProviderSurface::OpenAIResponses => OpenAIResponsesCodec.decode(request),
ProviderSurface::AnthropicMessages => AnthropicMessagesCodec.decode(request),
}
.ok()
let obj = request.content.as_object()?;
let descriptor = REGISTRY.iter().find(|d| (d.detect_request)(obj, None))?;
(descriptor.decode_request)(request).ok()
}
/// Best-effort decode of a raw response into [`AnnotatedLlmResponse`] (fail-open).
#[must_use]
pub fn normalize_response(raw: &Json) -> Option<AnnotatedLlmResponse> {
match detect_response_surface(raw)? {
ProviderSurface::OpenAIChat => OpenAIChatCodec.decode_response(raw),
ProviderSurface::OpenAIResponses => OpenAIResponsesCodec.decode_response(raw),
ProviderSurface::AnthropicMessages => AnthropicMessagesCodec.decode_response(raw),
}
.ok()
let obj = raw.as_object()?;
let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
let descriptor = match (matches.next(), matches.next()) {
(Some(descriptor), None) => descriptor,
_ => return None,
};
(descriptor.decode_response)(raw).ok()
fn unique_response_descriptor(obj: &serde_json::Map<String, Json>) -> Option<&'static SurfaceDescriptor> {
let mut matches = REGISTRY.iter().filter(|d| (d.detect_response)(obj));
match (matches.next(), matches.next()) {
(Some(descriptor), None) => Some(descriptor),
_ => None,
}
}
pub fn detect_response_surface(raw: &Json) -> Option<ProviderSurface> {
let obj = raw.as_object()?;
unique_response_descriptor(obj).map(|d| d.surface)
}
/// Best-effort decode of a raw request into [`AnnotatedLlmRequest`] (fail-open).
#[must_use]
pub fn normalize_request(request: &LlmRequest) -> Option<AnnotatedLlmRequest> {
let obj = request.content.as_object()?;
let descriptor = REGISTRY.iter().find(|d| (d.detect_request)(obj, None))?;
(descriptor.decode_request)(request).ok()
}
/// Best-effort decode of a raw response into [`AnnotatedLlmResponse`] (fail-open).
#[must_use]
pub fn normalize_response(raw: &Json) -> Option<AnnotatedLlmResponse> {
let obj = raw.as_object()?;
let descriptor = unique_response_descriptor(obj)?;
(descriptor.decode_response)(raw).ok()
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core/src/codec/resolve.rs` around lines 82 - 106, The exact-one
response descriptor selection rule is duplicated between detect_response_surface
and normalize_response, so extract that matching logic into a single private
helper and have both functions use it. Keep the helper responsible for iterating
REGISTRY and returning the sole matching descriptor only when there is exactly
one match, then reuse it in both detect_response_surface and normalize_response
so any future change to matching behavior stays consistent.

Comment thread crates/core/tests/unit/codec/resolve_tests.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement improvement to existing functionality lang:rust PR changes/introduces Rust code size:M PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant