Skip to content

chore: address Sonar code quality findings #738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rapids-bot merged 4 commits into from
Aug 7, 2026
Merged

chore: address Sonar code quality findings #738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d70a13f
chore: address Sonar code quality findings
willkill07 Aug 7, 2026
8246308
chore: reduce Gemini tool group complexity
willkill07 Aug 7, 2026
97ea3d8
fix: release pending Node subscriber callbacks
willkill07 Aug 7, 2026
89fecac
fix: address CodeRabbit review feedback
willkill07 Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 45 additions & 35 deletions crates/adaptive/src/response_cache/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,45 +453,55 @@ fn lossy_gemini_part(part: &Json, plain_text_parts: &mut usize) -> bool {
return true;
};
match data_key {
"text" => {
if !object.get("text").is_some_and(Json::is_string) {
return true;
}
if object.len() == 1 {
*plain_text_parts += 1;
"text" => lossy_gemini_text_part(object, plain_text_parts),
"functionCall" => lossy_gemini_function_call_part(object),
"functionResponse" => lossy_gemini_function_response_part(object),
_ => false,
}
}

fn lossy_gemini_text_part(
object: &serde_json::Map<String, Json>,
plain_text_parts: &mut usize,
) -> bool {
if !object.get("text").is_some_and(Json::is_string) {
return true;
}
if object.len() == 1 {
*plain_text_parts += 1;
}
false
}

fn lossy_gemini_function_call_part(object: &serde_json::Map<String, Json>) -> bool {
object.keys().any(|key| key != "functionCall")
|| match object.get("functionCall").and_then(Json::as_object) {
Some(call) => {
call.keys()
.any(|key| !matches!(key.as_str(), "name" | "id" | "args"))
|| call.get("args").is_some_and(|args| !args.is_object())
}
false
None => true,
}
"functionCall" => {
object.keys().any(|key| key != "functionCall")
|| match object.get("functionCall").and_then(Json::as_object) {
Some(fc) => {
fc.keys()
.any(|key| !matches!(key.as_str(), "name" | "id" | "args"))
|| fc.get("args").is_some_and(|args| !args.is_object())
}
None => true,
}
}
"functionResponse" => {
object.keys().any(|key| key != "functionResponse")
|| match object.get("functionResponse").and_then(Json::as_object) {
Some(fr) => {
fr.keys().any(|key| {
!matches!(key.as_str(), "id" | "name" | "response" | "parts")
}) || match (
fr.get("id").and_then(Json::as_str),
fr.get("name").and_then(Json::as_str),
) {
(Some(id), Some(name)) => id != name,
_ => false,
}
}

fn lossy_gemini_function_response_part(object: &serde_json::Map<String, Json>) -> bool {
object.keys().any(|key| key != "functionResponse")
|| match object.get("functionResponse").and_then(Json::as_object) {
Some(response) => {
response
.keys()
.any(|key| !matches!(key.as_str(), "id" | "name" | "response" | "parts"))
|| match (
response.get("id").and_then(Json::as_str),
response.get("name").and_then(Json::as_str),
) {
(Some(id), Some(name)) => id != name,
_ => false,
}
None => true,
}
}
None => true,
}
_ => false,
}
}

fn is_gemini_part_data_key(key: &str) -> bool {
Expand Down
248 changes: 132 additions & 116 deletions crates/cli/src/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,84 +888,80 @@ async fn observability_http_exporter_checks(
return Vec::new();
};
join_all(
endpoints
.iter()
.enumerate()
.map(|(index, endpoint)| async move {
let endpoint_type = endpoint
.get("type")
.and_then(Value::as_str)
.unwrap_or("unknown");
let label = "OpenTelemetry endpoint";
let transport = endpoint
.get("transport")
.and_then(Value::as_str)
.unwrap_or("http_binary");
match endpoint.get("endpoint").and_then(Value::as_str) {
Some(url) => {
let mut check = if transport == "grpc" {
if probe_mode.is_offline() {
match validate_grpc_endpoint(url) {
Ok(_) => Check {
name: label,
status: Status::Info,
details: format!(
"endpoints[{index}] ({endpoint_type}): live network probe skipped (--offline)"
),
},
Err(details) => Check {
name: label,
status: Status::Fail,
details: format!(
"endpoints[{index}] ({endpoint_type}): {details}"
),
},
}
} else {
probe_tcp_named(label, url).await
}
} else {
let effective_url = resolve_http_trace_endpoint(url);
if probe_mode.is_offline() {
match validate_otlp_http_endpoint(effective_url.as_ref()) {
Ok(()) => Check {
name: label,
status: Status::Info,
details: format!(
"endpoints[{index}] ({endpoint_type}): live network probe skipped (--offline)"
),
},
Err(details) => Check {
name: label,
status: Status::Fail,
details: format!(
"endpoints[{index}] ({endpoint_type}): {details}"
),
},
}
} else {
probe_otlp_http_named(label, effective_url.as_ref()).await
}
};
if !probe_mode.is_offline() {
check.details =
format!("endpoints[{index}] ({endpoint_type}): {}", check.details);
}
check
}
None => Check {
name: label,
status: Status::Fail,
details: format!(
"endpoints[{index}] ({endpoint_type}): endpoint is required"
),
},
}
}),
endpoints.iter().enumerate().map(|(index, endpoint)| {
observability_http_exporter_check(index, endpoint, probe_mode)
}),
)
.await
}

async fn observability_http_exporter_check(
index: usize,
endpoint: &Value,
probe_mode: DoctorProbeMode,
) -> Check {
let endpoint_type = endpoint
.get("type")
.and_then(Value::as_str)
.unwrap_or("unknown");
let label = "OpenTelemetry endpoint";
let Some(url) = endpoint.get("endpoint").and_then(Value::as_str) else {
return Check {
name: label,
status: Status::Fail,
details: format!("endpoints[{index}] ({endpoint_type}): endpoint is required"),
};
};
let transport = endpoint
.get("transport")
.and_then(Value::as_str)
.unwrap_or("http_binary");
let mut check = if transport == "grpc" {
probe_grpc_endpoint(label, url, probe_mode).await
} else {
probe_http_endpoint(label, url, probe_mode).await
};
check.details = format!("endpoints[{index}] ({endpoint_type}): {}", check.details);
check
}

async fn probe_grpc_endpoint(label: &'static str, url: &str, mode: DoctorProbeMode) -> Check {
if mode.is_offline() {
return match validate_grpc_endpoint(url) {
Ok(_) => Check {
name: label,
status: Status::Info,
details: "live network probe skipped (--offline)".into(),
},
Err(details) => Check {
name: label,
status: Status::Fail,
details,
},
};
}
probe_tcp_named(label, url).await
}

async fn probe_http_endpoint(label: &'static str, url: &str, mode: DoctorProbeMode) -> Check {
let effective_url = resolve_http_trace_endpoint(url);
if mode.is_offline() {
return match validate_otlp_http_endpoint(effective_url.as_ref()) {
Ok(()) => Check {
name: label,
status: Status::Info,
details: "live network probe skipped (--offline)".into(),
},
Err(details) => Check {
name: label,
status: Status::Fail,
details,
},
};
}
probe_otlp_http_named(label, effective_url.as_ref()).await
}

fn observability_component_config(plugin_value: &Value) -> Option<&Value> {
plugin_value
.get("components")
Expand Down Expand Up @@ -1229,54 +1225,74 @@ fn validate_atof_stream_probe_target(
fn endpoint_headers(endpoint: &Value) -> Result<Vec<(String, String)>, String> {
let mut out = Vec::new();
let mut names = std::collections::HashSet::new();
if let Some(headers) = endpoint.get("headers") {
let Some(object) = headers.as_object() else {
return Err("headers must be an object of string values".into());
append_configured_headers(endpoint.get("headers"), &mut names, &mut out)?;
append_environment_headers(endpoint.get("header_env"), &mut names, &mut out)?;
Ok(out)
}

fn append_configured_headers(
headers: Option<&Value>,
names: &mut std::collections::HashSet<reqwest::header::HeaderName>,
out: &mut Vec<(String, String)>,
) -> Result<(), String> {
let Some(headers) = headers else {
return Ok(());
};
let Some(object) = headers.as_object() else {
return Err("headers must be an object of string values".into());
};
for (key, value) in object {
let name = reqwest::header::HeaderName::from_bytes(key.as_bytes())
.map_err(|error| error.to_string())?;
let Some(value) = value.as_str() else {
return Err(format!("headers.{key} must be a string"));
};
for (key, value) in object {
let name = reqwest::header::HeaderName::from_bytes(key.as_bytes())
.map_err(|error| error.to_string())?;
let Some(value) = value.as_str() else {
return Err(format!("headers.{key} must be a string"));
};
if value.trim().is_empty() {
return Err(format!("headers.{key} must not be blank"));
}
reqwest::header::HeaderValue::from_bytes(value.as_bytes())
.map_err(|error| format!("headers.{key} invalid: {error}"))?;
if !names.insert(name) {
return Err(format!("header {key:?} appears more than once"));
}
out.push((key.clone(), value.to_string()));
if value.trim().is_empty() {
return Err(format!("headers.{key} must not be blank"));
}
reqwest::header::HeaderValue::from_bytes(value.as_bytes())
.map_err(|error| format!("headers.{key} invalid: {error}"))?;
if !names.insert(name) {
return Err(format!("header {key:?} appears more than once"));
}
out.push((key.clone(), value.to_string()));
}
if let Some(header_env) = endpoint.get("header_env") {
let Some(object) = header_env.as_object() else {
return Err("header_env must be an object of string values".into());
Ok(())
}

fn append_environment_headers(
header_env: Option<&Value>,
names: &mut std::collections::HashSet<reqwest::header::HeaderName>,
out: &mut Vec<(String, String)>,
) -> Result<(), String> {
let Some(header_env) = header_env else {
return Ok(());
};
let Some(object) = header_env.as_object() else {
return Err("header_env must be an object of string values".into());
};
for (key, variable) in object {
let name = reqwest::header::HeaderName::from_bytes(key.as_bytes())
.map_err(|error| error.to_string())?;
if names.contains(&name) {
return Err(format!(
"header {key:?} cannot appear in both headers and header_env"
));
}
let Some(variable) = variable.as_str() else {
return Err(format!("header_env.{key} must be a string"));
};
for (key, variable) in object {
let name = reqwest::header::HeaderName::from_bytes(key.as_bytes())
.map_err(|error| error.to_string())?;
if names.contains(&name) {
return Err(format!(
"header {key:?} cannot appear in both headers and header_env"
));
}
let Some(variable) = variable.as_str() else {
return Err(format!("header_env.{key} must be a string"));
};
let value = std::env::var(variable)
.map_err(|_| format!("environment variable {variable:?} is not set"))?;
if value.trim().is_empty() {
return Err(format!("environment variable {variable:?} is blank"));
}
reqwest::header::HeaderValue::from_bytes(value.as_bytes())
.map_err(|error| format!("header_env.{key} invalid: {error}"))?;
names.insert(name);
out.push((key.clone(), value));
let value = std::env::var(variable)
.map_err(|_| format!("environment variable {variable:?} is not set"))?;
if value.trim().is_empty() {
return Err(format!("environment variable {variable:?} is blank"));
}
reqwest::header::HeaderValue::from_bytes(value.as_bytes())
.map_err(|error| format!("header_env.{key} invalid: {error}"))?;
names.insert(name);
out.push((key.clone(), value));
}
Ok(out)
Ok(())
}

fn doctor_atof_probe_payload() -> Result<String, String> {
Expand Down
19 changes: 13 additions & 6 deletions crates/cli/tests/coverage/shared/doctor_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,11 +1201,16 @@ async fn opentelemetry_doctor_skips_live_network_probes_offline() {

assert_eq!(checks.len(), 2);
assert!(checks.iter().all(|check| check.status == Status::Info));
assert!(checks.iter().all(|check| {
check
assert!(
checks[0]
.details
.contains("live network probe skipped (--offline)")
}));
.contains("endpoints[0] (gen_ai): live network probe skipped (--offline)")
);
assert!(
checks[1]
.details
.contains("endpoints[1] (openinference): live network probe skipped (--offline)")
);
}

#[tokio::test]
Expand All @@ -1229,8 +1234,10 @@ async fn opentelemetry_doctor_offline_still_rejects_malformed_endpoints() {
.await;

assert_eq!(checks.len(), 2);
assert!(checks[0].details.contains("invalid gRPC endpoint"));
assert!(checks[1].details.contains("invalid OTLP HTTP endpoint"));
assert!(checks[0].details.starts_with("endpoints[0] (gen_ai): "));
assert!(checks[0].details.contains("gRPC endpoint"));
assert!(checks[1].details.starts_with("endpoints[1] (full): "));
assert!(checks[1].details.contains("OTLP HTTP endpoint"));
assert!(checks.iter().all(|check| check.status == Status::Fail));
}

Expand Down
Loading
Loading