Implement character-class matching in web model-scope globs - #16
Conversation
CodeRabbit review on #15 (web/server/modelScope.ts:61): patterns like zai/glm-5.[23] were classified as globs (hasGlobCharacter includes `[`) but globToRegExp escaped the brackets into literals, so the pattern matched the literal text "[23]" instead of 2-or-3 — hiding valid models with no fallback, since the glob path bypasses partial matching. parseCharClass now translates classes with minimatch's semantics: - member sets ([23]), ranges ([2-3]), ! and ^ negation - a ] directly after [ (or the negation) is a literal member - backslash escapes a class member - an unterminated [ falls back to a literal bracket - classes never match the path separator: / members are unreachable (dropped) and negated classes exclude / alongside their members Verified against real minimatch (nocase) across 216 pattern/model combinations; the only intentional divergences are the documented :thinking suffix stripping and the non-glob partial-match fallback. New unit tests cover matching and non-matching classes, negation, ranges, slash exclusion, unterminated classes, and leading-] members.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 25 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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. Comment |
Summary
Addresses the unresolved CodeRabbit review comment from #15 (
web/server/modelScope.ts:61, 🟠 Major · Functional Correctness): character classes were classified as globs but translated to literals.The bug:
hasGlobCharactercounts[as a glob character, butglobToRegExpescaped brackets into literals — sozai/glm-5.[23]matched the literal text[23]instead of2-or-3, hiding valid models from the picker with no recovery (the glob path bypasses the partial-match fallback).Changes
New
parseCharClasstranslates[...]with minimatch's semantics:[23]), ranges ([2-3]),!/^negation]directly after[(or the negation) is a literal member ([]23])[falls back to a literal bracket/members are unreachable (dropped) and negated classes exclude/alongside their membersVerification
minimatch(nocase) across 216 pattern × model combinations — agreement everywhere except the intentional divergences: documented:thinking-suffix stripping and the non-glob partial-match fallbacktests/web-model-scope.test.ts): matching/non-matching classes, negation + slash exclusion, unterminated classes, leading-]membersbun run checkand biome clean