fix(pkg):do regex precompile insteadd on the fly

[ItsT0ng]

📝 Description

• Move HTTP status regex compilation from per-call in extractHTTPStatus to package-level var init, avoiding repeated regexp.MustCompile on every error classification
• Fix the HTTP regex to use lowercase http since ClassifyError lowercases the input before calling extractHTTPStatus (the uppercase HTTP pattern was a dead code path)
• Add a standalone word-boundary status code matcher (\b([3-5]\d{2})\b) as a fallback for bare status codes like "error 429"
• Rename the package-level variable from patterns to httpStatusPatterns for clarity and to avoid shadowing the matchesAny parameter

Pattern priority

extractHTTPStatus now tries patterns in order of specificity:

1. status[:\s]+(\d{3}) — most specific, matches "status: 429" / "status 429"
2. http[/\s]+...(\d{3}) — matches "http/1.1 502" with protocol prefix
3. \b([3-5]\d{2})\b — standalone fallback, matches any bare 3xx–5xx code at a word boundary

Test changes

• Fix test input from "HTTP/1.1 502 Bad Gateway" to "http/1.1 502 bad gateway" to match the actual lowercased calling convention
• Add test case for standalone status code extraction ("error 429" → 429)

🗣️ Type of Change

• 🐞 Bug fix (non-breaking change which fixes an issue)
• ✨ New feature (non-breaking change which adds functionality)
• 📖 Documentation update
• ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

• 🤖 Fully AI-generated (100% AI, 0% Human)
• 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
• 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

• Reference URL:
• Reasoning:

🧪 Test Environment

• Hardware: PC
• OS: Ubuntu 22.04
• Model/Provider: N/A
• Channels: N/A

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

• My code/docs follow the style of this project.
• I have performed a self-review of my own changes.
• I have updated the documentation accordingly.

[Copilot]

Pull request overview

This PR refactors the provider error classifier to avoid recompiling HTTP-status regexes on the hot path (extractHTTPStatus), improving performance when classifying frequent LLM API errors.

Changes:

• Moved HTTP-status regexp.MustCompile calls from inside extractHTTPStatus to a package-level precompiled slice.
• Updated extractHTTPStatus to iterate over the shared precompiled regex list.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

---

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

[nikolasdehor]

Review: fix(pkg/providers): do regex precompile instead of on the fly

Good performance optimization with a correctness fix included.

Changes

1. error_classifier.go -- Moved httpStatusPatterns from local variables inside extractHTTPStatus() to package-level var block. This avoids recompiling 2-3 regexes on every error classification call. Since regexp.MustCompile is safe for concurrent use after compilation, this is the correct Go pattern.

2. Fixed case mismatch -- Changed HTTP[/\s]+ to http[/\s]+ in the regex. This is a real bug fix: ClassifyError lowercases the input before calling extractHTTPStatus, so the uppercase HTTP pattern was dead code that could never match. Good catch.

3. Added standalone status code fallback -- New pattern \b([3-5]\d{2})\b matches bare status codes like "error 429". This improves error classification coverage for messages that don't follow the "status: NNN" or "http/1.1 NNN" format.

4. Test updates -- Fixed test input to use lowercase (matching actual calling convention) and added "error 429" test case.

shell.go Changes

5. absolutePathPattern moved to package-level -- Same precompile optimization. Previously compiled inside guardCommand() on every call.

6. defaultDenyPatterns reformatted from var = []*regexp.Regexp{...} to var (\n defaultDenyPatterns = ...\n absolutePathPattern = ...\n) block -- This is just grouping, no functional change.

Concern

7. Standalone status code regex \b([3-5]\d{2})\b is broad -- This will match any 3-digit number from 300-599 at a word boundary. Strings like "connection on port 443" would return 443 as a status code. "error code 500" is fine, but "timeout after 300ms" would incorrectly match 300. Consider narrowing the pattern or adding it only as a last-resort fallback with lower priority (which it already is, being the last pattern tried). The current test cases pass, but edge cases could produce false positives.

8. httpStatusPatterns variable rename -- Changed from patterns to httpStatusPatterns. Good -- avoids shadowing the patterns parameter name in matchesAny. Clean naming.

LGTM with the caveat on point #7 (broad fallback pattern). The precompile optimization is correct and the case-sensitivity fix addresses a real bug.

[xiaket]

LGTM, thanks!