Problem
collect_messages_from_for() (crates/mds-core/src/evaluator.rs:833-922) re-implements the entire @for driver logic that text mode already factors into evaluate_for → evaluate_for_key_value/evaluate_for_array → run_loop_body.
Both paths duplicate:
- Key/array dispatch logic
MAX_LOOP_ITERATIONS and MAX_TOTAL_ITERATIONS guards
- Alphabetical key-sort determinism
- Push/bind/pop body loop mechanics
The two implementations must stay in sync by hand or they silently diverge.
Risk
This is core iteration logic. If refactored incorrectly, messages could be silently dropped. The logic is correct and fully tested today, but maintaining it as two separate codepaths is fragile.
Suggested Approach
Extract a generic iteration driver or body-callback closure that both evaluate_for_* and collect_messages_from_for reuse:
- Option A: Generic iteration driver
for_each_loop_bindings<F> accepting a callback F for accumulation
- Option B: Refactor to a shared
run_loop_body that can accumulate into either String or Vec<EvalMessage>
Acceptance Criteria
Notes
Originally review finding I2, complexity/architecture, MEDIUM confidence it's duplication. Deferred from PR #87 in favor of a focused refactoring PR with full test coverage.
Related: PR #87 (Messages mode feature), commits 2754d52 and 532ac03
Problem
collect_messages_from_for()(crates/mds-core/src/evaluator.rs:833-922) re-implements the entire@fordriver logic that text mode already factors intoevaluate_for→evaluate_for_key_value/evaluate_for_array→run_loop_body.Both paths duplicate:
MAX_LOOP_ITERATIONSandMAX_TOTAL_ITERATIONSguardsThe two implementations must stay in sync by hand or they silently diverge.
Risk
This is core iteration logic. If refactored incorrectly, messages could be silently dropped. The logic is correct and fully tested today, but maintaining it as two separate codepaths is fragile.
Suggested Approach
Extract a generic iteration driver or body-callback closure that both
evaluate_for_*andcollect_messages_from_forreuse:for_each_loop_bindings<F>accepting a callbackFfor accumulationrun_loop_bodythat can accumulate into eitherStringorVec<EvalMessage>Acceptance Criteria
@fortests pass (text, messages, key-value, arrays, limits)cargo clippy— 0 warnings@forexecutionNotes
Originally review finding I2, complexity/architecture, MEDIUM confidence it's duplication. Deferred from PR #87 in favor of a focused refactoring PR with full test coverage.
Related: PR #87 (Messages mode feature), commits 2754d52 and 532ac03