Skip to content

Commit 3c9bb84

Browse files
committed
refactor: extract helper functions and improve code organization
- README: Update tagline to clarify declarative nature - command.rs: Extract test helper functions for better readability - retry.rs: Split complex functions into smaller, focused helpers - parsers.rs: Refactor git status parsing with dedicated functions
1 parent dc85682 commit 3c9bb84

File tree

4 files changed

+193
-147
lines changed

4 files changed

+193
-147
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
[![Security](https://github.com/iepathos/mmm/actions/workflows/security.yml/badge.svg)](https://github.com/iepathos/mmm/actions/workflows/security.yml)
55
[![Release](https://github.com/iepathos/mmm/actions/workflows/release.yml/badge.svg)](https://github.com/iepathos/mmm/actions/workflows/release.yml)
66

7-
Define AI-assisted development workflows in YAML. Run bounded, testable improvement loops. Ship better code.
7+
Define declarative AI development workflows in YAML. Run bounded, testable improvement loops. Ship better code.
88

99
## What Is MMM?
1010

src/config/command.rs

Lines changed: 52 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,15 @@ commit_required: false
802802

803803
#[test]
804804
fn test_workflow_config_with_new_syntax() {
805-
// Test parsing the exact structure used in coverage.yml
805+
let config = parse_test_workflow_config();
806+
assert_eq!(config.commands.len(), 3);
807+
808+
verify_coverage_command(&config.commands[0]);
809+
verify_implement_spec_command(&config.commands[1]);
810+
verify_lint_command(&config.commands[2]);
811+
}
812+
813+
fn parse_test_workflow_config() -> WorkflowConfig {
806814
let yaml = r#"
807815
commands:
808816
- claude: "/mmm-coverage"
@@ -820,65 +828,72 @@ commands:
820828
commit_required: false
821829
"#;
822830

823-
let config: WorkflowConfig = match serde_yaml::from_str(yaml) {
824-
Ok(c) => c,
825-
Err(e) => {
826-
// Try to parse just the commands array to debug
827-
let yaml_value: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap();
828-
if let Some(commands) = yaml_value.get("commands") {
829-
println!("Commands value: {commands:?}");
830-
831-
// Try to parse each command
832-
if let Some(seq) = commands.as_sequence() {
833-
for (i, cmd) in seq.iter().enumerate() {
834-
println!("\nCommand {i}: {cmd:?}");
835-
match serde_yaml::from_value::<WorkflowStepCommand>(cmd.clone()) {
836-
Ok(_parsed) => println!(" Parsed as WorkflowStepCommand: success"),
837-
Err(e2) => println!(" Failed as WorkflowStepCommand: {e2}"),
838-
}
839-
match serde_yaml::from_value::<WorkflowCommand>(cmd.clone()) {
840-
Ok(parsed) => println!(" Parsed as WorkflowCommand: {parsed:?}"),
841-
Err(e2) => println!(" Failed as WorkflowCommand: {e2}"),
842-
}
843-
}
844-
}
845-
}
846-
panic!("Failed to parse WorkflowConfig: {e}");
831+
serde_yaml::from_str(yaml).unwrap_or_else(|e| {
832+
debug_workflow_parsing_error(yaml, &e);
833+
panic!("Failed to parse WorkflowConfig: {e}");
834+
})
835+
}
836+
837+
fn debug_workflow_parsing_error(yaml: &str, _error: &serde_yaml::Error) {
838+
let yaml_value: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap();
839+
if let Some(commands) = yaml_value.get("commands") {
840+
println!("Commands value: {commands:?}");
841+
if let Some(seq) = commands.as_sequence() {
842+
debug_command_sequence(seq);
847843
}
848-
};
849-
assert_eq!(config.commands.len(), 3);
844+
}
845+
}
846+
847+
fn debug_command_sequence(seq: &[serde_yaml::Value]) {
848+
for (i, cmd) in seq.iter().enumerate() {
849+
println!("\nCommand {i}: {cmd:?}");
850+
try_parse_as::<WorkflowStepCommand>(cmd, "WorkflowStepCommand");
851+
try_parse_as::<WorkflowCommand>(cmd, "WorkflowCommand");
852+
}
853+
}
850854

851-
// Verify first command
852-
match &config.commands[0] {
855+
fn try_parse_as<T: serde::de::DeserializeOwned + std::fmt::Debug>(
856+
value: &serde_yaml::Value,
857+
type_name: &str,
858+
) {
859+
match serde_yaml::from_value::<T>(value.clone()) {
860+
Ok(parsed) => println!(" Parsed as {type_name}: {parsed:?}"),
861+
Err(e) => println!(" Failed as {type_name}: {e}"),
862+
}
863+
}
864+
865+
fn verify_coverage_command(command: &WorkflowCommand) {
866+
match command {
853867
WorkflowCommand::WorkflowStep(step) => {
854868
assert_eq!(step.claude, Some("/mmm-coverage".to_string()));
855869
assert_eq!(step.id, Some("coverage".to_string()));
856870
assert!(!step.commit_required);
857871
assert!(step.outputs.is_some());
858872
assert!(step.analysis.is_some());
859873
}
860-
_ => panic!("Expected WorkflowStep variant for first command"),
874+
_ => panic!("Expected WorkflowStep variant for coverage command"),
861875
}
876+
}
862877

863-
// Verify second command
864-
match &config.commands[1] {
878+
fn verify_implement_spec_command(command: &WorkflowCommand) {
879+
match command {
865880
WorkflowCommand::WorkflowStep(step) => {
866881
assert_eq!(
867882
step.claude,
868883
Some("/mmm-implement-spec ${coverage.spec}".to_string())
869884
);
870-
// inputs removed - arguments now passed directly in command string
871885
}
872-
_ => panic!("Expected WorkflowStep variant for second command"),
886+
_ => panic!("Expected WorkflowStep variant for implement-spec command"),
873887
}
888+
}
874889

875-
// Verify third command
876-
match &config.commands[2] {
890+
fn verify_lint_command(command: &WorkflowCommand) {
891+
match command {
877892
WorkflowCommand::WorkflowStep(step) => {
878893
assert_eq!(step.claude, Some("/mmm-lint".to_string()));
879894
assert!(!step.commit_required);
880895
}
881-
_ => panic!("Expected WorkflowStep variant for third command"),
896+
_ => panic!("Expected WorkflowStep variant for lint command"),
882897
}
883898
}
884899
}

src/cook/retry.rs

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -38,54 +38,29 @@ pub async fn execute_with_retry(
3838

3939
while attempt <= max_retries {
4040
if attempt > 0 {
41-
let delay = Duration::from_secs(2u64.pow(attempt.min(3))); // Exponential backoff, max 8s
42-
if verbose {
43-
println!(
44-
"⏳ Retrying {description} after {delay:?} (attempt {attempt}/{max_retries})"
45-
);
46-
}
47-
sleep(delay).await;
41+
await_retry_delay(attempt, description, max_retries, verbose).await;
4842
}
4943

5044
match command.output().await {
5145
Ok(output) => {
52-
// Check if it's a transient error we should retry
53-
if !output.status.success() {
54-
let stderr = String::from_utf8_lossy(&output.stderr);
55-
56-
// Retry on specific error conditions
57-
if is_transient_error(&stderr) && attempt < max_retries {
58-
if verbose {
59-
eprintln!(
60-
"⚠️ Transient error detected: {}",
61-
stderr.lines().next().unwrap_or("Unknown error")
62-
);
63-
}
64-
last_error = Some(stderr.to_string());
65-
attempt += 1;
66-
continue;
46+
if let Some(retry_reason) = should_retry_output(&output, attempt, max_retries) {
47+
if verbose {
48+
eprintln!("⚠️ Transient error detected: {retry_reason}");
6749
}
50+
last_error = Some(retry_reason);
51+
attempt += 1;
52+
continue;
6853
}
69-
7054
return Ok(output);
7155
}
7256
Err(e) => {
73-
// System-level errors (e.g., command not found) shouldn't be retried
74-
if e.kind() == std::io::ErrorKind::NotFound {
75-
return Err(e).context(format!("Command not found for {description}"));
76-
}
77-
78-
// Other IO errors might be transient
79-
if attempt < max_retries {
80-
if verbose {
81-
eprintln!("⚠️ IO error: {e}");
82-
}
83-
last_error = Some(e.to_string());
57+
let retry_result =
58+
handle_command_error(e, description, attempt, max_retries, verbose)?;
59+
if let Some(error_msg) = retry_result {
60+
last_error = Some(error_msg);
8461
attempt += 1;
8562
continue;
8663
}
87-
88-
return Err(e).context(format!("Failed to execute {description}"));
8964
}
9065
}
9166
}
@@ -98,6 +73,51 @@ pub async fn execute_with_retry(
9873
))
9974
}
10075

76+
async fn await_retry_delay(attempt: u32, description: &str, max_retries: u32, verbose: bool) {
77+
let delay = Duration::from_secs(2u64.pow(attempt.min(3))); // Exponential backoff, max 8s
78+
if verbose {
79+
println!("⏳ Retrying {description} after {delay:?} (attempt {attempt}/{max_retries})");
80+
}
81+
sleep(delay).await;
82+
}
83+
84+
fn should_retry_output(
85+
output: &std::process::Output,
86+
attempt: u32,
87+
max_retries: u32,
88+
) -> Option<String> {
89+
if !output.status.success() {
90+
let stderr = String::from_utf8_lossy(&output.stderr);
91+
if is_transient_error(&stderr) && attempt < max_retries {
92+
return Some(stderr.lines().next().unwrap_or("Unknown error").to_string());
93+
}
94+
}
95+
None
96+
}
97+
98+
fn handle_command_error(
99+
error: std::io::Error,
100+
description: &str,
101+
attempt: u32,
102+
max_retries: u32,
103+
verbose: bool,
104+
) -> Result<Option<String>> {
105+
// System-level errors (e.g., command not found) shouldn't be retried
106+
if error.kind() == std::io::ErrorKind::NotFound {
107+
return Err(error).context(format!("Command not found for {description}"));
108+
}
109+
110+
// Other IO errors might be transient
111+
if attempt < max_retries {
112+
if verbose {
113+
eprintln!("⚠️ IO error: {error}");
114+
}
115+
return Ok(Some(error.to_string()));
116+
}
117+
118+
Err(error).context(format!("Failed to execute {description}"))
119+
}
120+
101121
/// Check if an error message indicates a transient failure
102122
///
103123
/// Detects common patterns that indicate temporary failures which

0 commit comments

Comments
 (0)