Refactor stdlib to use generic helper functions

[logbie]

Summary of Changes

• The Issue: src/stdlib/text.rs and src/stdlib/list.rs contained repetitive boilerplate code for argument count validation and type extraction (e.g., check_arg_count followed by expect_text or expect_list).
• The Rational: This redundancy violated the DRY principle and made the code harder to maintain. Consolidating this logic into helper functions improves readability and maintainability.
• The Solution: Implemented generic helper functions in src/stdlib/helpers.rs (unary_text_op, binary_text_predicate, unary_list_action, binary_list_action, unary_list_op) and refactored the standard library modules to use them.

Verification Checklist

• cargo fmt executed and passed.
• cargo clippy returned no warnings or errors.
• All cargo test suites passed (100% success rate).

---

PR created automatically by Jules for task 3168250324251832024 started by @logbie

Summary by CodeRabbit

• Refactor
  • Standardized implementation patterns for text and list operations in the standard library to improve code consistency and maintainability across similar operations.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[claude]

Claude finished @logbie's task —— View job

---

Code Review Complete

I've conducted a thorough review of this PR refactoring stdlib modules to use generic helper functions. Overall, this is a well-executed refactoring that significantly improves code quality and maintainability.

Todo List:

• Read repository guidelines from CLAUDE.md
• Analyze the new generic helper functions in src/stdlib/helpers.rs
• Review refactored src/stdlib/text.rs implementation
• Review refactored src/stdlib/list.rs implementation
• Check for potential bugs or issues
• Evaluate performance implications
• Assess security considerations
• Review test coverage implications
• Provide comprehensive feedback and recommendations

---

✅ Positive Aspects

Excellent DRY Principle Implementation

The refactoring successfully eliminates repetitive boilerplate code across text and list modules. The generic helper functions in src/stdlib/helpers.rs provide a clean, reusable pattern for:

• Argument validation (check_arg_count, check_min_arg_count, check_arg_range)
• Type extraction (expect_text, expect_list, expect_number, etc.)
• Operation patterns (unary_text_op, binary_text_predicate, unary_list_action, etc.)

Strong Type Safety & Error Handling

The helper functions maintain robust error handling with:

• Clear, user-friendly error messages with proper singular/plural grammar
• Consistent error formatting across all stdlib functions
• Proper type validation before operations

Performance Considerations

• Memory Efficiency: Uses Arc<str> and Rc<RefCell<Vec<Value>>> for efficient memory sharing
• Zero-copy Operations: Text operations use references where possible
• Cloning Optimization: Smart use of Arc::clone() for reference counting instead of data duplication

---

🔍 Issues Found

1. Type Inconsistency in expect_text() - src/stdlib/helpers.rs:328

pub fn expect_text(value: &Value) -> Result<Arc<str>, RuntimeError> {
    match value {
        Value::Text(s) => Ok(Arc::clone(s)), // Returns Arc<str>

Issue: The function documentation at line 302 mentions Rc<str> but the implementation uses Arc<str>. This should be consistent.

Recommendation: Update documentation to reflect Arc<str> usage or standardize on one reference counting type throughout the codebase.

2. Potential Performance Impact in unique() - src/stdlib/list.rs:232

let key = format!("{:?}:{}", std::mem::discriminant(item), item);

Issue: Creating formatted strings for each item during uniqueness checking could be expensive for large lists with complex values.

Recommendation: Consider using a more efficient hashing strategy or caching discriminants.

---

🛡️ Security Assessment

✅ No Security Concerns Found

• Input validation is properly maintained
• No unsafe operations introduced
• Memory management follows Rust safety patterns
• No potential for buffer overflows or injection attacks

---

🧪 Test Coverage Assessment

✅ Excellent Test Coverage

• Both text.rs and list.rs maintain comprehensive test suites
• Tests cover edge cases (empty inputs, bounds checking, error conditions)
• Unicode handling properly tested (e.g., test_last_index_of_unicode_char_index)
• Error conditions well-covered (negative/infinite values, out-of-bounds access)

Recommendation: Consider adding integration tests specifically for the new helper functions to ensure they work correctly in isolation.

---

📊 Code Quality Assessment

Strengths

• Consistent Naming: All helper functions follow clear naming conventions
• Comprehensive Documentation: Excellent docstrings with examples for all helper functions
• Error Handling: Robust and user-friendly error messages
• Code Organization: Logical grouping of helper functions by purpose

Minor Improvements

• Consider extracting common constants like MAX_PAD_WIDTH to a module-level constants section
• Some functions in text.rs could benefit from additional inline comments for complex Unicode handling

---

🎯 Performance Impact

Overall: Neutral to Positive

• Positive: Reduced code duplication should improve compilation times
• Positive: Better code locality may improve CPU cache utilization
• Neutral: Runtime performance should remain essentially unchanged
• Minor Concern: The unique() function implementation could be optimized for better performance on large datasets

---

📝 Recommendations

High Priority

1. Fix Documentation Consistency: Update expect_text() documentation to reflect Arc<str> usage

Medium Priority

1. Optimize unique() Performance: Consider more efficient uniqueness checking for large lists
2. Add Helper Function Tests: Include unit tests specifically for the new generic helpers

Low Priority

1. Code Organization: Group related constants at module level
2. Documentation Enhancement: Add performance characteristics to helper function docs

---

✨ Summary

This is a high-quality refactoring that successfully achieves its goals of reducing code duplication and improving maintainability. The implementation follows Rust best practices and maintains the existing API contracts. The few minor issues identified don't impact functionality but would enhance code quality if addressed.

Recommendation: ✅ APPROVE - This PR significantly improves the codebase with minimal risk and should be merged after addressing the documentation inconsistency.

---

[Copilot]

Pull request overview

This PR refactors the standard library to eliminate repetitive boilerplate code by introducing generic helper functions. The changes consolidate common patterns of argument validation and type extraction into reusable helpers, improving code maintainability and adherence to the DRY principle.

Changes:

• Added five new generic helper functions (unary_text_op, binary_text_predicate, unary_list_action, binary_list_action, unary_list_op) in src/stdlib/helpers.rs
• Refactored text operations in src/stdlib/text.rs to use the new helpers
• Refactored list operations in src/stdlib/list.rs to use the new helpers

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/stdlib/helpers.rs	Implements generic helper functions for text and list operations
src/stdlib/text.rs	Refactors text functions to use unary_text_op and binary_text_predicate helpers
src/stdlib/list.rs	Refactors list functions to use unary_list_action, binary_list_action, and unary_list_op helpers

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

This PR introduces five generic helper functions in helpers.rs for data-type operations and refactors the text and list stdlib modules to use these shared helpers, consolidating argument validation and type extraction logic while maintaining existing behavior.

Changes

Cohort / File(s)	Summary
Helper Functions src/stdlib/helpers.rs	Added five new public generic helper functions: unary_text_op, binary_text_predicate, unary_list_action, binary_list_action, and unary_list_op to encapsulate common argument validation, type extraction, and operation application patterns.
Text Operations src/stdlib/text.rs	Refactored text functions (touppercase, tolowercase, trim, capitalize, reverse, starts_with, ends_with) to use unary_text_op and binary_text_predicate helpers, removing manual argument checking and text extraction boilerplate.
List Operations src/stdlib/list.rs	Refactored list functions (push, unshift, clear, fill, sort, reverse_list, pop, shift) to use binary_list_action and unary_list_op helpers, consolidating borrow_mut logic and boundary checks into reusable patterns.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• Refactor stdlib argument validation helpers #311: Modifies src/stdlib/helpers.rs and refactors text/list stdlib functions using shared helper utilities
• Refactor math stdlib to use generic helper functions #352: Introduces generic helper functions in src/stdlib/helpers.rs and applies them across stdlib modules (math helpers pattern)
• Refactor text stdlib to use generic helper functions #346: Refactors the same text stdlib functions (touppercase, tolowercase, etc.) using unary_text_op/binary_text_predicate helpers

Poem

🐰 Five helpers hop with joyful cheer,
Text and lists now crystal clear!
Boilerplate banished, patterns shine,
Generic magic works just fine! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main change: refactoring stdlib modules to use newly introduced generic helper functions instead of repetitive boilerplate code.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/stdlib-helpers-3168250324251832024

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[logbie]

Closing in triage: this PR is currently conflicted (DIRTY) and superseded by newer clean PRs in the same optimization/refactor sequence. Reopen or rebase if you want this exact change revived.

[google-labs-jules]

Closing in triage: this PR is currently conflicted (DIRTY) and superseded by newer clean PRs in the same optimization/refactor sequence. Reopen or rebase if you want this exact change revived.

Understood. Acknowledging that this work is now superseded and stopping work on this task.