Skip to content

Commit 2d9b4f4

Browse files
authored
refactor: reduce sonar cognitive complexity (NVIDIA#167)
#### Overview Refactor the functions reported by Sonar for excessive cognitive complexity while preserving runtime behavior. The changes split branch-heavy Rust and Go test logic into focused helpers. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Extracted plugin editor menu action handling into smaller helpers. - Extracted CLI session event routing and explicit subagent alias handling. - Split ATIF agent scope tree construction into focused construction, linking, and sorting helpers. - Split ATIF test shape assertions into targeted helper assertions. - Split Go observability and OpenInference tests into setup and assertion helpers. Validation run: - `cargo fmt --all` - `gofmt` - `cargo clippy --workspace --all-targets -- -D warnings` - `just test-go` - `go vet ./...` - `just test-node` - `just test-wasm` - `cargo test -p nemo-relay-cli --bin nemo-relay -- --test-threads=1` Notes: - `just test-rust` compiled the affected crates and passed the core/adaptive suites, but hit an existing cwd-sensitive parallel CLI test failure. The failing test passed in isolation and in the serial CLI binary suite listed above. - `just test-python` is blocked by dependency resolution for `nemo-relay[deepagents]`: the resolver only sees `langchain<=1.3.2` for the Linux split while the existing project requirements ask for `langchain>=1.3.3`. #### Where should the reviewer start? Start with `crates/core/src/observability/atif.rs` for the largest decomposition, then `crates/cli/src/session.rs` for the session routing helper extraction. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: none Authors: - Will Killian (https://github.com/willkill07) Approvers: - David Gardner (https://github.com/dagardner-nv) URL: NVIDIA#167
1 parent 0fd08c0 commit 2d9b4f4

6 files changed

Lines changed: 527 additions & 302 deletions

File tree

crates/cli/src/plugins.rs

Lines changed: 149 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use console::{Key, Term, style};
1414
use dialoguer::theme::ColorfulTheme;
1515
use dialoguer::{Input, Select};
1616
use nemo_relay::config_editor::{EditorFieldKind, EditorFieldSpec};
17+
use nemo_relay::plugin::PluginConfig;
1718
use serde_json::{Value, json};
1819

1920
use crate::config::PluginsEditCommand;
@@ -41,6 +42,12 @@ enum MenuResponse {
4142
Cancel,
4243
}
4344

45+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
46+
enum EditLoopControl {
47+
Continue,
48+
Finish,
49+
}
50+
4451
#[derive(Debug)]
4552
struct MenuItem {
4653
label: String,
@@ -107,82 +114,152 @@ pub(crate) fn edit(command: PluginsEditCommand) -> Result<(), CliError> {
107114
if let Some(selected) = menu_response_index(&selection) {
108115
selected_index = selected;
109116
}
110-
match selection {
111-
MenuResponse::Selected(selection) => match actions.get(selection).copied() {
112-
Some(MenuAction::ToggleComponent(component_index)) => {
113-
if let Some(component) = components.get_mut(component_index) {
114-
component.toggle_enabled();
115-
}
116-
}
117-
Some(MenuAction::EditField {
118-
component_index,
119-
field_index,
120-
}) => {
121-
if let Some(component) = components.get_mut(component_index)
122-
&& let Some(field) = component.fields().get(field_index)
123-
{
124-
edit_component_field(&theme, component, *field)?;
125-
}
126-
}
127-
Some(MenuAction::Preview) => {
128-
let preview_config = config_with_editable_components(&config, &components)?;
129-
print_preview(&preview_config)?;
130-
}
131-
Some(MenuAction::Save) => {
132-
store_editable_components(&mut config, &components)?;
133-
validate_config(&config)?;
134-
write_plugin_config(&path, &config)?;
135-
print_save_success(&path);
136-
return Ok(());
137-
}
138-
Some(MenuAction::Cancel) | None => {
139-
return Err(CliError::Config(
140-
"plugin edit cancelled; no config saved".into(),
141-
));
142-
}
143-
},
144-
MenuResponse::Shortcut(MenuShortcut::Preview, _) => {
145-
let preview_config = config_with_editable_components(&config, &components)?;
146-
print_preview(&preview_config)?;
147-
}
148-
MenuResponse::Shortcut(MenuShortcut::Save, _) => {
149-
store_editable_components(&mut config, &components)?;
150-
validate_config(&config)?;
151-
write_plugin_config(&path, &config)?;
152-
print_save_success(&path);
153-
return Ok(());
117+
if handle_menu_response(
118+
&theme,
119+
&path,
120+
&mut config,
121+
&mut components,
122+
&actions,
123+
selection,
124+
)? == EditLoopControl::Finish
125+
{
126+
return Ok(());
127+
}
128+
}
129+
}
130+
131+
fn handle_menu_response(
132+
theme: &ColorfulTheme,
133+
path: &Path,
134+
config: &mut PluginConfig,
135+
components: &mut [EditableComponent],
136+
actions: &[MenuAction],
137+
selection: MenuResponse,
138+
) -> Result<EditLoopControl, CliError> {
139+
match selection {
140+
MenuResponse::Selected(selection) => handle_menu_action(
141+
theme,
142+
path,
143+
config,
144+
components,
145+
actions.get(selection).copied(),
146+
),
147+
MenuResponse::Shortcut(MenuShortcut::Preview, _) => {
148+
preview_components(config, components)?;
149+
Ok(EditLoopControl::Continue)
150+
}
151+
MenuResponse::Shortcut(MenuShortcut::Save, _) => save_components(path, config, components),
152+
MenuResponse::Shortcut(MenuShortcut::Help, _) => {
153+
print_editor_help();
154+
Ok(EditLoopControl::Continue)
155+
}
156+
MenuResponse::Shortcut(
157+
shortcut @ (MenuShortcut::Reset | MenuShortcut::Clear),
158+
selected,
159+
) => handle_reset_or_clear_shortcut(components, actions.get(selected).copied(), shortcut),
160+
MenuResponse::Cancel => Err(cancelled_error()),
161+
}
162+
}
163+
164+
fn handle_menu_action(
165+
theme: &ColorfulTheme,
166+
path: &Path,
167+
config: &mut PluginConfig,
168+
components: &mut [EditableComponent],
169+
action: Option<MenuAction>,
170+
) -> Result<EditLoopControl, CliError> {
171+
match action {
172+
Some(MenuAction::ToggleComponent(component_index)) => {
173+
if let Some(component) = components.get_mut(component_index) {
174+
component.toggle_enabled();
154175
}
155-
MenuResponse::Shortcut(MenuShortcut::Help, _) => print_editor_help(),
156-
MenuResponse::Shortcut(
157-
shortcut @ (MenuShortcut::Reset | MenuShortcut::Clear),
158-
selected,
159-
) => match actions.get(selected).copied() {
160-
Some(MenuAction::ToggleComponent(component_index)) => {
161-
if let Some(component) = components.get_mut(component_index) {
162-
apply_component_enablement_shortcut(component, shortcut);
163-
}
164-
}
165-
Some(MenuAction::EditField {
166-
component_index,
167-
field_index,
168-
}) => {
169-
if let Some(component) = components.get_mut(component_index)
170-
&& let Some(field) = component.fields().get(field_index)
171-
{
172-
component.reset_field(*field)?;
173-
}
174-
}
175-
_ => {
176-
println!(" Select a component or editable field to reset or clear.");
177-
}
178-
},
179-
MenuResponse::Cancel => {
180-
return Err(CliError::Config(
181-
"plugin edit cancelled; no config saved".into(),
182-
));
176+
Ok(EditLoopControl::Continue)
177+
}
178+
Some(MenuAction::EditField {
179+
component_index,
180+
field_index,
181+
}) => {
182+
edit_selected_component_field(theme, components, component_index, field_index)?;
183+
Ok(EditLoopControl::Continue)
184+
}
185+
Some(MenuAction::Preview) => {
186+
preview_components(config, components)?;
187+
Ok(EditLoopControl::Continue)
188+
}
189+
Some(MenuAction::Save) => save_components(path, config, components),
190+
Some(MenuAction::Cancel) | None => Err(cancelled_error()),
191+
}
192+
}
193+
194+
fn edit_selected_component_field(
195+
theme: &ColorfulTheme,
196+
components: &mut [EditableComponent],
197+
component_index: usize,
198+
field_index: usize,
199+
) -> Result<(), CliError> {
200+
if let Some(component) = components.get_mut(component_index)
201+
&& let Some(field) = component.fields().get(field_index)
202+
{
203+
edit_component_field(theme, component, *field)?;
204+
}
205+
Ok(())
206+
}
207+
208+
fn preview_components(
209+
config: &PluginConfig,
210+
components: &[EditableComponent],
211+
) -> Result<(), CliError> {
212+
let preview_config = config_with_editable_components(config, components)?;
213+
print_preview(&preview_config)
214+
}
215+
216+
fn save_components(
217+
path: &Path,
218+
config: &mut PluginConfig,
219+
components: &[EditableComponent],
220+
) -> Result<EditLoopControl, CliError> {
221+
store_editable_components(config, components)?;
222+
validate_config(config)?;
223+
write_plugin_config(path, config)?;
224+
print_save_success(path);
225+
Ok(EditLoopControl::Finish)
226+
}
227+
228+
fn handle_reset_or_clear_shortcut(
229+
components: &mut [EditableComponent],
230+
action: Option<MenuAction>,
231+
shortcut: MenuShortcut,
232+
) -> Result<EditLoopControl, CliError> {
233+
match action {
234+
Some(MenuAction::ToggleComponent(component_index)) => {
235+
if let Some(component) = components.get_mut(component_index) {
236+
apply_component_enablement_shortcut(component, shortcut);
183237
}
184238
}
239+
Some(MenuAction::EditField {
240+
component_index,
241+
field_index,
242+
}) => reset_selected_component_field(components, component_index, field_index)?,
243+
_ => println!(" Select a component or editable field to reset or clear."),
185244
}
245+
Ok(EditLoopControl::Continue)
246+
}
247+
248+
fn reset_selected_component_field(
249+
components: &mut [EditableComponent],
250+
component_index: usize,
251+
field_index: usize,
252+
) -> Result<(), CliError> {
253+
if let Some(component) = components.get_mut(component_index)
254+
&& let Some(field) = component.fields().get(field_index)
255+
{
256+
component.reset_field(*field)?;
257+
}
258+
Ok(())
259+
}
260+
261+
fn cancelled_error() -> CliError {
262+
CliError::Config("plugin edit cancelled; no config saved".into())
186263
}
187264

188265
fn edit_component_field(

crates/cli/src/session.rs

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -277,27 +277,11 @@ impl SessionManager {
277277
continue;
278278
}
279279

280-
let mut event = alignment_state.route_event(event);
281-
let explicit_subagent_alias = alignment::explicit_subagent_alias(&mut event);
282-
let session_id = event.session_id().to_string();
283-
let is_agent_started = matches!(&event, NormalizedEvent::AgentStarted(_));
284-
if event.is_terminal() && !sessions.contains_key(&session_id) {
280+
let Some((event, session_id, is_agent_started)) =
281+
route_event_for_session(event, &mut sessions, &mut alignment_state)
282+
else {
285283
continue;
286-
}
287-
if let Some((child_session_id, alias)) = explicit_subagent_alias {
288-
if sessions
289-
.get(&child_session_id)
290-
.is_none_or(|session| session.can_reparent_as_subagent_alias())
291-
{
292-
sessions.remove(&child_session_id);
293-
alignment_state.insert_alias(child_session_id, alias);
294-
alignment_state.align_explicit_subagent_end(&mut event);
295-
} else {
296-
continue;
297-
}
298-
} else {
299-
alignment_state.align_explicit_subagent_end(&mut event);
300-
}
284+
};
301285
let event_kind = event_agent_kind(&event);
302286
let should_remove_session = apply_event_to_session(
303287
&mut sessions,
@@ -756,6 +740,53 @@ async fn close_idle_sessions_from_parts(
756740
first_error.map_or(Ok(closed_turns), Err)
757741
}
758742

743+
fn route_event_for_session(
744+
event: NormalizedEvent,
745+
sessions: &mut HashMap<String, Session>,
746+
alignment_state: &mut SessionAlignmentState,
747+
) -> Option<(NormalizedEvent, String, bool)> {
748+
let mut event = alignment_state.route_event(event);
749+
let explicit_subagent_alias = alignment::explicit_subagent_alias(&mut event);
750+
let session_id = event.session_id().to_string();
751+
let is_agent_started = matches!(&event, NormalizedEvent::AgentStarted(_));
752+
753+
if event.is_terminal() && !sessions.contains_key(&session_id) {
754+
return None;
755+
}
756+
if !apply_explicit_subagent_alias(
757+
&mut event,
758+
sessions,
759+
alignment_state,
760+
explicit_subagent_alias,
761+
) {
762+
return None;
763+
}
764+
Some((event, session_id, is_agent_started))
765+
}
766+
767+
fn apply_explicit_subagent_alias(
768+
event: &mut NormalizedEvent,
769+
sessions: &mut HashMap<String, Session>,
770+
alignment_state: &mut SessionAlignmentState,
771+
explicit_subagent_alias: Option<(String, SessionAlias)>,
772+
) -> bool {
773+
let Some((child_session_id, alias)) = explicit_subagent_alias else {
774+
alignment_state.align_explicit_subagent_end(event);
775+
return true;
776+
};
777+
778+
if sessions
779+
.get(&child_session_id)
780+
.is_some_and(|session| !session.can_reparent_as_subagent_alias())
781+
{
782+
return false;
783+
}
784+
sessions.remove(&child_session_id);
785+
alignment_state.insert_alias(child_session_id, alias);
786+
alignment_state.align_explicit_subagent_end(event);
787+
true
788+
}
789+
759790
impl Session {
760791
// Constructs per-session runtime state without creating a scope yet. The root agent scope is
761792
// opened lazily on the first event or gateway LLM call so sessions created from hints and pure

0 commit comments

Comments
 (0)