From 67db378bd04b8e4c476b386473b6b7bd34bb5222 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 23 Mar 2026 16:07:26 -0400 Subject: [PATCH] Remove dead selected_options field from Answer struct The field was populated in constructors but never read by any code. Selected keys are already carried by AnswerValue::MultiSelected(Vec), making this field redundant. Also removes the unused options parameter from Answer::multi_selected(). Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-api/src/server.rs | 15 ++++++--------- lib/crates/fabro-cli/src/commands/attach.rs | 3 --- lib/crates/fabro-cli/tests/cli.rs | 2 +- lib/crates/fabro-interview/src/auto_approve.rs | 1 - lib/crates/fabro-interview/src/console.rs | 9 +-------- lib/crates/fabro-interview/src/lib.rs | 12 +----------- lib/crates/fabro-workflows/tests/integration.rs | 6 ------ 7 files changed, 9 insertions(+), 39 deletions(-) diff --git a/lib/crates/fabro-api/src/server.rs b/lib/crates/fabro-api/src/server.rs index db3817df7..4bce2f7bf 100644 --- a/lib/crates/fabro-api/src/server.rs +++ b/lib/crates/fabro-api/src/server.rs @@ -858,18 +858,15 @@ async fn submit_answer( } else if !req.selected_option_keys.is_empty() { let pending = interviewer.pending_questions(); let pq = pending.iter().find(|pq| pq.id == qid); - let mut options = Vec::new(); for key in &req.selected_option_keys { - let opt = pq - .and_then(|pq| pq.question.options.iter().find(|o| o.key == *key).cloned()); - match opt { - Some(o) => options.push(o), - None => { - return ApiError::bad_request("Invalid option key.").into_response(); - } + let valid = pq + .and_then(|pq| pq.question.options.iter().find(|o| o.key == *key)) + .is_some(); + if !valid { + return ApiError::bad_request("Invalid option key.").into_response(); } } - Answer::multi_selected(req.selected_option_keys, options) + Answer::multi_selected(req.selected_option_keys) } else if let Some(v) = req.value { Answer::text(v) } else { diff --git a/lib/crates/fabro-cli/src/commands/attach.rs b/lib/crates/fabro-cli/src/commands/attach.rs index 7c697e99a..be9363cda 100644 --- a/lib/crates/fabro-cli/src/commands/attach.rs +++ b/lib/crates/fabro-cli/src/commands/attach.rs @@ -493,13 +493,11 @@ mod tests { let aborted = Answer { value: AnswerValue::Aborted, selected_option: None, - selected_options: Vec::new(), text: None, }; let skipped = Answer { value: AnswerValue::Skipped, selected_option: None, - selected_options: Vec::new(), text: None, }; let answered = Answer::yes(); @@ -561,7 +559,6 @@ mod tests { let answer = Answer { value: AnswerValue::Text("ship it".to_string()), selected_option: None, - selected_options: Vec::new(), text: Some("ship it".to_string()), }; diff --git a/lib/crates/fabro-cli/tests/cli.rs b/lib/crates/fabro-cli/tests/cli.rs index c29328f5a..53d03b9b2 100644 --- a/lib/crates/fabro-cli/tests/cli.rs +++ b/lib/crates/fabro-cli/tests/cli.rs @@ -1035,7 +1035,7 @@ fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { "question_type": "YesNo", "options": [], "allow_freeform": false, - "default": {"value": "Yes", "selected_option": null, "selected_options": [], "text": null}, + "default": {"value": "Yes", "selected_option": null, "text": null}, "timeout_seconds": 1.0, "stage": "gate", "metadata": {} diff --git a/lib/crates/fabro-interview/src/auto_approve.rs b/lib/crates/fabro-interview/src/auto_approve.rs index feca03330..4265080c7 100644 --- a/lib/crates/fabro-interview/src/auto_approve.rs +++ b/lib/crates/fabro-interview/src/auto_approve.rs @@ -16,7 +16,6 @@ impl Interviewer for AutoApproveInterviewer { |first| Answer { value: AnswerValue::Selected(first.key.clone()), selected_option: Some(first.clone()), - selected_options: Vec::new(), text: None, }, ) diff --git a/lib/crates/fabro-interview/src/console.rs b/lib/crates/fabro-interview/src/console.rs index 53110545c..556881d66 100644 --- a/lib/crates/fabro-interview/src/console.rs +++ b/lib/crates/fabro-interview/src/console.rs @@ -34,7 +34,6 @@ fn find_matching_option(response: &str, options: &[QuestionOption]) -> Option Option Answer { Answer { value: AnswerValue::Selected(opt.key.clone()), selected_option: Some(opt.clone()), - selected_options: Vec::new(), text: None, } } @@ -174,11 +171,7 @@ fn ask_multi_select_interactive(question: &Question) -> Answer { .iter() .map(|&i| question.options[i].key.clone()) .collect(); - let options: Vec<_> = indices - .iter() - .map(|&i| question.options[i].clone()) - .collect(); - Answer::multi_selected(keys, options) + Answer::multi_selected(keys) } _ => Answer::aborted(), } diff --git a/lib/crates/fabro-interview/src/lib.rs b/lib/crates/fabro-interview/src/lib.rs index c6c1a39a1..5a058df2f 100644 --- a/lib/crates/fabro-interview/src/lib.rs +++ b/lib/crates/fabro-interview/src/lib.rs @@ -90,8 +90,6 @@ pub enum AnswerValue { pub struct Answer { pub value: AnswerValue, pub selected_option: Option, - #[serde(default)] - pub selected_options: Vec, pub text: Option, } @@ -101,7 +99,6 @@ impl Answer { Self { value: AnswerValue::Yes, selected_option: None, - selected_options: Vec::new(), text: None, } } @@ -111,7 +108,6 @@ impl Answer { Self { value: AnswerValue::No, selected_option: None, - selected_options: Vec::new(), text: None, } } @@ -121,7 +117,6 @@ impl Answer { Self { value: AnswerValue::Aborted, selected_option: None, - selected_options: Vec::new(), text: None, } } @@ -131,7 +126,6 @@ impl Answer { Self { value: AnswerValue::Skipped, selected_option: None, - selected_options: Vec::new(), text: None, } } @@ -141,7 +135,6 @@ impl Answer { Self { value: AnswerValue::Timeout, selected_option: None, - selected_options: Vec::new(), text: None, } } @@ -151,16 +144,14 @@ impl Answer { Self { value: AnswerValue::Selected(key), selected_option: Some(option), - selected_options: Vec::new(), text: None, } } - pub fn multi_selected(keys: Vec, options: Vec) -> Self { + pub fn multi_selected(keys: Vec) -> Self { Self { value: AnswerValue::MultiSelected(keys), selected_option: None, - selected_options: options, text: None, } } @@ -170,7 +161,6 @@ impl Answer { Self { value: AnswerValue::Text(t.clone()), selected_option: None, - selected_options: Vec::new(), text: Some(t), } } diff --git a/lib/crates/fabro-workflows/tests/integration.rs b/lib/crates/fabro-workflows/tests/integration.rs index c3c58db18..bbe6d3ca5 100644 --- a/lib/crates/fabro-workflows/tests/integration.rs +++ b/lib/crates/fabro-workflows/tests/integration.rs @@ -452,7 +452,6 @@ async fn end_to_end_human_gate_pipeline() { let answers = VecDeque::from([Answer { value: AnswerValue::Selected("R".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }]); let interviewer = Arc::new(QueueInterviewer::new(answers)); @@ -2523,13 +2522,11 @@ async fn human_gate_loops_back() { Answer { value: AnswerValue::Selected("F".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }, Answer { value: AnswerValue::Selected("A".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }, ]); @@ -7013,7 +7010,6 @@ async fn human_gate_freeform_with_fixed_choice_match() { let answers = VecDeque::from([Answer { value: AnswerValue::Selected("A".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }]); let interviewer = Arc::new(QueueInterviewer::new(answers)); @@ -7267,7 +7263,6 @@ async fn human_gate_freeform_sets_allow_freeform_on_question() { let answers = VecDeque::from([Answer { value: AnswerValue::Selected("A".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }]); let inner = QueueInterviewer::new(answers); @@ -7382,7 +7377,6 @@ async fn human_gate_without_freeform_sets_allow_freeform_false() { let answers = VecDeque::from([Answer { value: AnswerValue::Selected("A".to_string()), selected_option: None, - selected_options: Vec::new(), text: None, }]); let inner = QueueInterviewer::new(answers);