-
Notifications
You must be signed in to change notification settings - Fork 0
feat(budgets): add wildcard pattern support for budget configuration #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Deploying modestbench with
|
| Latest commit: |
97bc957
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a529fc4d.modestbench.pages.dev |
| Branch Preview URL: | https://feature-budget-wildcards.modestbench.pages.dev |
44638eb to
e9b0df2
Compare
There was a problem hiding this comment.
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 adds wildcard pattern support to the budget configuration system, allowing users to apply performance budgets broadly using glob patterns for files and simple * wildcards for suite and task names.
Key changes:
- Introduces
BudgetPatternandResolvedBudgetstypes to separate exact matches from pattern-based budgets - Implements a budget resolution service with minimatch-based file pattern matching and specificity-based precedence
- Fixes budget evaluation to properly handle tasks with both absolute and relative budgets when no baseline is provided
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/budgets.ts | Adds BudgetPattern and ResolvedBudgets type definitions |
| src/types/core.ts | Exports new budget types |
| src/services/budget-resolver.ts | New service implementing pattern matching, specificity calculation, and budget merging logic |
| src/services/budget-evaluator.ts | Updates to use ResolvedBudgets and fix handling of mixed absolute/relative budgets without baseline |
| src/config/schema.ts | Transforms nested budget config into ResolvedBudgets structure with exact matches and patterns separated |
| src/core/engine.ts | Updates engine integration to handle ResolvedBudgets structure |
| test/unit/services/budget-resolver.test.ts | Comprehensive unit tests for budget resolution logic |
| test/unit/services/budget-evaluator.test.ts | Updated tests to use ResolvedBudgets format |
| test/unit/config/budget-wildcards.test.ts | Unit tests for wildcard budget configuration parsing |
| test/unit/config/budget-config-integration.test.ts | Updates integration tests for ResolvedBudgets format |
| test/integration/budget-wildcards.test.ts | End-to-end integration tests for wildcard pattern evaluation |
| site/src/content/docs/guides/performance-budgets.mdx | Documentation for wildcard patterns with examples and precedence rules |
| README.md | Updated budget configuration examples to show wildcard support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
…mple - Update specificity scoring table to match actual implementation: - Exact file: +2 (was +3) - Glob with specific parts: +1 (was +2 for partial, +1 for full) - Suite: +1 (was +2) - Full glob/wildcard: +0 - Fix duplicate 'Authentication' key in JSON example that would cause first entry to be silently overwritten - Update example calculations to reflect correct scoring (max 4 pts) Addresses PR #201 review comments from @copilot-pull-request-reviewer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dff2f44 to
97bc957
Compare
Add support for glob patterns in file paths and simple wildcards (`*`) for suite and task names in budget configuration. This allows users to apply budgets to multiple tasks at once without enumerating each one. Key changes: - Add BudgetPattern and ResolvedBudgets types for pattern-based budgets - Create budget-resolver service with minimatch file matching and simple wildcard suite/task matching - Update transformBudgets() to produce ResolvedBudgets with exact matches and patterns separated - Update BudgetEvaluator to resolve budgets using pattern matching with specificity-based precedence - Fix budget evaluation to not skip tasks with both absolute and relative budgets when no baseline is provided Pattern specificity (higher = more specific): - Exact file/suite/task: 4 - Exact file + exact suite + wildcard task: 3 - Glob file + exact suite/task: 3 - Exact file + wildcard suite/task: 2 - Glob file + wildcard suite/task: 1 - Full wildcard (**/* / * / *): 0 When multiple patterns match, budgets are merged with more specific values overriding less specific ones. Closes #72 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…mple - Update specificity scoring table to match actual implementation: - Exact file: +2 (was +3) - Glob with specific parts: +1 (was +2 for partial, +1 for full) - Suite: +1 (was +2) - Full glob/wildcard: +0 - Fix duplicate 'Authentication' key in JSON example that would cause first entry to be silently overwritten - Update example calculations to reflect correct scoring (max 4 pts) Addresses PR #201 review comments from @copilot-pull-request-reviewer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add Performance Budgets, Profiling, and Test Adapters guides to the documentation sidebar - these pages existed but weren't accessible via navigation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
97bc957 to
9de14ce
Compare
…mple - Update specificity scoring table to match actual implementation: - Exact file: +2 (was +3) - Glob with specific parts: +1 (was +2 for partial, +1 for full) - Suite: +1 (was +2) - Full glob/wildcard: +0 - Fix duplicate 'Authentication' key in JSON example that would cause first entry to be silently overwritten - Update example calculations to reflect correct scoring (max 4 pts) Addresses PR #201 review comments from @copilot-pull-request-reviewer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const parseTaskId = ( | ||
| taskId: TaskId, | ||
| ): { file: string; suite: string; task: string } => { | ||
| const str = taskId as string; | ||
| const lastSlash = str.lastIndexOf('/'); | ||
| const secondLastSlash = str.lastIndexOf('/', lastSlash - 1); | ||
|
|
||
| return { | ||
| file: str.substring(0, secondLastSlash), | ||
| suite: str.substring(secondLastSlash + 1, lastSlash), | ||
| task: str.substring(lastSlash + 1), | ||
| }; | ||
| }; |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parseTaskId function doesn't handle edge cases where a TaskId might have fewer than two slashes. If a malformed TaskId is passed (e.g., "test.bench.js/taskName" with only one slash, or "taskName" with no slashes), the function will return incorrect results:
- For "test.bench.js/taskName":
secondLastSlashwould be -1, causingfileto be empty string,suiteto be "test.bench.js", andtaskto be "taskName" - For "taskName": both would be -1, causing negative substring indices
While TaskIds should always be properly formatted in normal usage, consider adding validation or handling for malformed input to prevent unexpected behavior, especially since this is an exported function that could be called directly.
| * A budget pattern with glob support for files and simple wildcards for | ||
| * suite/task | ||
| * | ||
| * File patterns use minimatch glob syntax (e.g., `**\/*.bench.js`). Suite/task |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backslash in the JSDoc comment is unnecessary and incorrect. In JSDoc and TypeScript comments, backticks do not need to be escaped. The comment currently shows **\/*.bench.js but should show **/*.bench.js to correctly represent the glob pattern syntax.
This same issue appears in the calculateSpecificity function documentation.
| * File patterns use minimatch glob syntax (e.g., `**\/*.bench.js`). Suite/task | |
| * File patterns use minimatch glob syntax (e.g., `**/*.bench.js`). Suite/task |
| const parsed = parseTaskId(taskId); | ||
| const matchingPatterns = budgets.patterns.filter( | ||
| (p) => | ||
| matchesFile(p.filePattern, parsed.file) && | ||
| matchesSuiteOrTask(p.suitePattern, parsed.suite) && | ||
| matchesSuiteOrTask(p.taskPattern, parsed.task), | ||
| ); | ||
|
|
||
| if (matchingPatterns.length === 0) { | ||
| return exactMatch; | ||
| } | ||
|
|
||
| // Sort by specificity ascending (least specific first, so more specific can override) | ||
| const sorted = [...matchingPatterns].sort( | ||
| (a, b) => a.specificity - b.specificity, | ||
| ); | ||
|
|
||
| // Merge all patterns, then apply exact match last | ||
| let merged = sorted[0]!.budget; | ||
| for (let i = 1; i < sorted.length; i++) { | ||
| merged = mergeBudgets(merged, sorted[i]!.budget); | ||
| } | ||
|
|
||
| // Exact match has highest priority | ||
| return mergeBudgets(merged, exactMatch); | ||
| } | ||
|
|
||
| // Find all matching patterns | ||
| const parsed = parseTaskId(taskId); | ||
| const matchingPatterns = budgets.patterns.filter( | ||
| (p) => | ||
| matchesFile(p.filePattern, parsed.file) && | ||
| matchesSuiteOrTask(p.suitePattern, parsed.suite) && | ||
| matchesSuiteOrTask(p.taskPattern, parsed.task), | ||
| ); | ||
|
|
||
| if (matchingPatterns.length === 0) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Sort by specificity ascending (least specific first) | ||
| const sorted = [...matchingPatterns].sort( | ||
| (a, b) => a.specificity - b.specificity, | ||
| ); | ||
|
|
||
| // Merge all matches (more specific overrides less specific) | ||
| let merged = sorted[0]!.budget; | ||
| for (let i = 1; i < sorted.length; i++) { | ||
| merged = mergeBudgets(merged, sorted[i]!.budget); | ||
| } | ||
|
|
||
| return merged; |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is significant code duplication between the exact match path (lines 174-198) and the no-exact-match path (lines 202-225). The logic for parsing TaskId, filtering matching patterns, sorting, and merging is identical in both branches.
Consider extracting this logic into a helper function to reduce duplication and improve maintainability. For example:
const resolveFromPatterns = (taskId: TaskId, patterns: readonly BudgetPattern[]): Budget | undefined => {
const parsed = parseTaskId(taskId);
const matchingPatterns = patterns.filter(
(p) =>
matchesFile(p.filePattern, parsed.file) &&
matchesSuiteOrTask(p.suitePattern, parsed.suite) &&
matchesSuiteOrTask(p.taskPattern, parsed.task),
);
if (matchingPatterns.length === 0) {
return undefined;
}
const sorted = [...matchingPatterns].sort(
(a, b) => a.specificity - b.specificity,
);
let merged = sorted[0]!.budget;
for (let i = 1; i < sorted.length; i++) {
merged = mergeBudgets(merged, sorted[i]!.budget);
}
return merged;
};Then the main function could be simplified to avoid repeating this logic.
| * @returns True if the pattern contains glob metacharacters | ||
| */ | ||
| export const isGlobPattern = (pattern: string): boolean => { | ||
| return /[*?[\]]/.test(pattern); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex pattern for detecting glob patterns is missing the curly brace metacharacter used in brace expansion (e.g., {a,b,c}). Minimatch supports this syntax, so patterns like *.{js,ts} or test.{bench,test}.js would not be detected as glob patterns by isGlobPattern(), even though they would match files using minimatch.
Consider updating the regex to include curly braces: /[*?[\]{}]/ or /[*?[\]{}]/ to properly detect all minimatch glob patterns.
| return /[*?[\]]/.test(pattern); | |
| return /[*?[\]{}]/.test(pattern); |

Add support for glob patterns in file paths and simple wildcards (
*)for suite and task names in budget configuration. This allows users to
apply budgets to multiple tasks at once without enumerating each one.
Key changes:
wildcard suite/task matching
and patterns separated
specificity-based precedence
budgets when no baseline is provided
Pattern specificity (higher = more specific):
When multiple patterns match, budgets are merged with more specific
values overriding less specific ones.
Closes #72
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]