Skip to content

Implement character-class matching in web model-scope globs - #16

Merged
ianwalter merged 1 commit into
mainfrom
fix/web-model-scope-char-classes
Aug 20, 2026
Merged

Implement character-class matching in web model-scope globs#16
ianwalter merged 1 commit into
mainfrom
fix/web-model-scope-char-classes

Conversation

@ianwalter

Copy link
Copy Markdown
Contributor

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: hasGlobCharacter counts [ as a glob character, but globToRegExp escaped brackets into literals — so zai/glm-5.[23] matched the literal text [23] instead of 2-or-3, hiding valid models from the picker with no recovery (the glob path bypasses the partial-match fallback).

Changes

New parseCharClass translates [...] with minimatch's semantics:

  • member sets ([23]), ranges ([2-3]), !/^ negation
  • a ] directly after [ (or the negation) is a literal member ([]23])
  • 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

Verification

  • Differential-tested against real minimatch (nocase) across 216 pattern × model combinations — agreement everywhere except the intentional divergences: documented :thinking-suffix stripping and the non-glob partial-match fallback
  • 4 new unit tests (12 total in tests/web-model-scope.test.ts): matching/non-matching classes, negation + slash exclusion, unterminated classes, leading-] members
  • Full suite: 365 pass / 0 fail; bun run check and biome clean

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.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ianwalter, you've reached your PR review limit, so we couldn't start this review.

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 @coderabbitai review or push new commits to the PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a364237f-d23d-4b63-b6df-7667b1b728f6

📥 Commits

Reviewing files that changed from the base of the PR and between 93e86bc and ddae60c.

📒 Files selected for processing (2)
  • tests/web-model-scope.test.ts
  • web/server/modelScope.ts

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.

@ianwalter
ianwalter merged commit 4e227c9 into main Aug 20, 2026
8 checks passed
@ianwalter
ianwalter deleted the fix/web-model-scope-char-classes branch August 20, 2026 03:35
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.

1 participant