Skip to content

⚡ Bolt: [performance improvement] Refactor pattern module manual argument extraction - #502

Closed
logbie wants to merge 1 commit into
mainfrom
perf/bolt-pattern-arg-extraction-3879846073613580161
Closed

⚡ Bolt: [performance improvement] Refactor pattern module manual argument extraction#502
logbie wants to merge 1 commit into
mainfrom
perf/bolt-pattern-arg-extraction-3879846073613580161

Conversation

@logbie

@logbie logbie commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Summary of Changes

  • The Issue: The src/stdlib/pattern.rs file had manual duplicate logic to pattern-match and validate Value enums (e.g. Value::Text and Value::Pattern). This was highly verbose and inefficient for checking argument types and returning custom RuntimeErrors.
  • The Rational: Unifying this logic through standard expect_* and check_arg_count helpers improves maintainability by adhering to the DRY principle, reduces redundant error instantiation, and makes the module much more concise, directly resolving technical debt.
  • The Solution: Added a new expect_pattern macro to src/stdlib/helpers.rs and migrated the pattern.rs functions (pattern_matches_native, pattern_find_native, pattern_find_all_native, native_pattern_replace, native_pattern_split) to use the consolidated extraction logic. Refactored the unit tests in src/stdlib/pattern_test.rs to check for the standardized error message Expected text and expects 2 arguments.

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 3879846073613580161 started by @logbie


Open in Devin Review

Summary by CodeRabbit

  • Refactor

    • Consolidated pattern validation logic using shared helper functions.
  • Bug Fixes

    • Standardized error messaging for pattern operations to provide consistent feedback.

Review Change Stack

…ment extraction

💡 What:
Replaced manual `match` blocks for extracting text and pattern arguments in `src/stdlib/pattern.rs` with the `expect_text` and newly added `expect_pattern` helpers from `src/stdlib/helpers.rs`.
Updated the corresponding tests in `src/stdlib/pattern_test.rs` to assert the standardized error messages produced by these helpers.

🎯 Why:
To eliminate duplicated argument validation and extraction logic, adhering to the DRY principle. This streamlines the codebase, improves maintainability, and prevents redundant memory allocations and cloning. Using the central `expect_*` macros allows consistent error formatting and simplifies the native function definitions in the pattern standard library.

📊 Impact:
Improved code maintainability and eliminated redundant `match` blocks. The argument validation logic is now unified across pattern functions, promoting reusability and reducing the size of individual functions.

🔬 Measurement:
Executed `cargo test` to verify that the pattern module tests still pass and correctly assert the new standardized error messages (e.g., 'Expected text'). Executed `cargo clippy` and `cargo fmt` to verify code quality.

Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 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.

Copilot AI review requested due to automatic review settings May 14, 2026 10:06
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: adbd2590-6cad-4815-b276-53f65ad4fa53

📥 Commits

Reviewing files that changed from the base of the PR and between 68d08ee and cca302e.

📒 Files selected for processing (3)
  • src/stdlib/helpers.rs
  • src/stdlib/pattern.rs
  • src/stdlib/pattern_test.rs

📝 Walkthrough

Walkthrough

This PR introduces a new expect_pattern helper extractor and refactors five pattern stdlib functions to use centralized argument validation helpers (check_arg_count, expect_text, expect_pattern) instead of manual checks, with test assertions updated to match the new standardized error messages.

Changes

Pattern Helper Consolidation

Layer / File(s) Summary
Expect pattern extractor
src/stdlib/helpers.rs
New expect_pattern function via generate_expect! macro extracts Value::Pattern and clones the underlying Rc<CompiledPattern>, returning type-mismatch errors with standardized wording.
Pattern functions refactored to use helpers
src/stdlib/pattern.rs
Five functions (pattern_matches_native, pattern_find_native, pattern_find_all_native, native_pattern_replace, native_pattern_split) replace manual args.len() checks and Value variant matching with check_arg_count and helper-based extraction; control flow and core logic remain identical.
Test assertions updated for standardized errors
src/stdlib/pattern_test.rs
Argument count error messages changed to "expects 2 arguments"; type error message changed to "Expected text" to align with helper-generated error text.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • WebFirstLanguage/wfl#310: Both PRs extend src/stdlib/helpers.rs to centralize typed Value extractors; main PR adds expect_pattern alongside prior expect_text/expect_number/expect_list helpers.
  • WebFirstLanguage/wfl#314: Both PRs refactor stdlib functions to use shared helpers-based argument validation and typed extractors (check_arg_count, expect_* patterns).
  • WebFirstLanguage/wfl#427: Both PRs use the generate_expect! macro in src/stdlib/helpers.rs to create typed expect_* extractors; retrieved PR establishes the macro, main PR applies it for patterns.

Poem

🐰 With helper hands so clean and true,
Five pattern functions now refactored through,
No more manual checks to parse—
Shared extractors save the day,
Tests aligned the modern way! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title references '[performance improvement]' and mentions 'refactor' of pattern module argument extraction, but the actual changes are primarily a code refactoring using helper functions, not a measurable performance improvement. Clarify whether this is truly a performance improvement or a code quality/maintainability refactor. Consider retitling to 'Refactor pattern module to use shared argument validation helpers' if the primary benefit is code consolidation rather than performance.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/bolt-pattern-arg-extraction-3879846073613580161

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the stdlib pattern module to remove duplicated manual Value matching/validation and replace it with shared helpers, standardizing argument/type error behavior across the module.

Changes:

  • Migrated pattern_* native functions in src/stdlib/pattern.rs to use check_arg_count, expect_text, and the new expect_pattern.
  • Added expect_pattern to src/stdlib/helpers.rs via the existing generate_expect! helper pattern.
  • Updated src/stdlib/pattern_test.rs assertions to match the standardized error message wording.

Reviewed changes

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

File Description
src/stdlib/pattern.rs Replaces manual argument/type extraction with shared stdlib helpers for consistency and reduced duplication.
src/stdlib/helpers.rs Adds expect_pattern extractor to support consolidated pattern argument validation.
src/stdlib/pattern_test.rs Aligns tests with the standardized “expects N arguments” / “Expected text” error messaging.

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

@logbie logbie closed this May 22, 2026
@logbie
logbie deleted the perf/bolt-pattern-arg-extraction-3879846073613580161 branch June 19, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants