Skip to content

fix(queryLanguage): allow parenthesized regex alternation in filter values#946

Merged
brendan-kellam merged 4 commits intomainfrom
brendan/fix-query-parser-paren-value
Feb 26, 2026
Merged

fix(queryLanguage): allow parenthesized regex alternation in filter values#946
brendan-kellam merged 4 commits intomainfrom
brendan/fix-query-parser-paren-value

Conversation

@brendan-kellam
Copy link
Contributor

@brendan-kellam brendan-kellam commented Feb 26, 2026

Summary

  • Queries like file:(test|spec), -file:(test|spec), repo:(org1|org2), and sym:(Foo|Bar) previously failed with a parse error because the word tokenizer unconditionally deferred to parenToken whenever a token started with balanced parentheses, even in filter value contexts
  • Fixed by detecting value context (preceding non-whitespace char is :) and using depth-tracking to consume the entire (...) as a word rather than deferring
  • Added 8 test cases covering filter alternation for file:, repo:, sym:, content:, negated alternation, and combined queries

Test plan

  • file:(test|spec) parses as PrefixExpr(FileExpr)
  • -file:(test|spec) parses as NegateExpr(PrefixExpr(FileExpr))
  • chat lang:TypeScript -file:(test|spec) parses successfully ✓
  • Existing paren grouping (foo bar) still works as ParenExpr
  • -(file:test or file:spec) still works as NegateExpr(ParenExpr(...))
  • All 248 existing tests pass ✓

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Search query parser now accepts parenthesized regex alternation in filter values (e.g., file:(test|spec), -file:(test|spec), repo:(org1|org2)).
  • Tests

    • Added test cases for alternation in prefix values, negated prefixes, and combined prefix scenarios.
  • Chores

    • Updated CI workflow definitions and job naming.

…alues

Queries like `file:(test|spec)` or `-file:(test|spec)` previously failed
with "No parse at N" because the word tokenizer unconditionally deferred
to parenToken whenever a token started with balanced parentheses, even in
value contexts (right after a prefix keyword colon like `file:`, `repo:`,
`sym:`, etc.).

The fix detects value context by looking backward for a preceding ':' and,
when found, uses depth-tracking to consume the entire '(...)' as a word
instead of deferring. This correctly handles nested parens, stops at an
outer ParenExpr closing paren, and leaves all existing parse behaviour
unchanged for non-value contexts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 26, 2026

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between ff4fa79 and aa44ab9.

📒 Files selected for processing (2)
  • .github/workflows/test-backend.yml
  • .github/workflows/test.yml

✏️ Tip: You can disable in-progress messages and the fortune message in your review settings.

Walkthrough

Adds support for parenthesized regex alternation in search filter values (e.g., file:(test|spec), -repo:(org1|org2)). The tokenizer distinguishes parentheses used for value contexts (after :) from grouping, and tests and changelog entries were added.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.md
Adds a Fixed entry documenting acceptance of parenthesized regex alternation in filter values.
Tokenizer Logic
packages/queryLanguage/src/tokens.ts
Updates wordToken to detect an opening ( in value contexts (preceded by :), consume balanced parentheses with depth tracking as part of the word token, and otherwise defer to existing paren tokenization.
Tests
packages/queryLanguage/test/prefixes.txt, packages/queryLanguage/test/negation.txt
Adds test cases covering parenthesized alternation in prefix values (file, repo, sym, content), combined prefix scenarios, and negated-prefix examples like `-file:(test

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: enabling parenthesized regex alternation in filter values.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch brendan/fix-query-parser-paren-value

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Co-authored-by: Brendan Kellam <brendan-kellam@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/queryLanguage/src/tokens.ts (1)

337-352: Consider handling escaped parentheses for consistency.

The depth-tracking loop doesn't skip escaped parentheses (e.g., \( or \)), while hasBalancedParensAt() at lines 124-127 does handle escapes. This inconsistency could cause unexpected behavior for patterns containing escaped parens like file:(test\)|spec).

If escaped parens in filter values are not a supported use case, this is fine. Otherwise, consider adding escape handling:

♻️ Proposed fix to handle escaped parens
         let depth = 0;
         while (input.next !== EOF) {
             const ch = input.next;
             if (isWhitespace(ch)) break;
+            // Handle escaped characters - skip the next character after a backslash
+            if (ch === 92 /* backslash */) {
+                input.advance();
+                if (input.next !== EOF) input.advance();
+                continue;
+            }
             if (ch === OPEN_PAREN) {
                 depth++;
             } else if (ch === CLOSE_PAREN) {
                 if (depth === 0) break; // outer ParenExpr closing — don't consume
                 depth--;
             }
             input.advance();
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/queryLanguage/src/tokens.ts` around lines 337 - 352, The
depth-tracking loop inside the inValueParenContext branch doesn't account for
escaped parens, causing mismatched depth when encountering \(' or \)'; update
the loop (the code using input.next, input.advance, OPEN_PAREN and CLOSE_PAREN)
to detect and skip escaped characters (e.g., when a backslash precedes a paren)
so escaped '(' or ')' do not change depth or terminate the loop, and ensure this
behavior mirrors hasBalancedParensAt's escape handling for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/queryLanguage/src/tokens.ts`:
- Around line 337-352: The depth-tracking loop inside the inValueParenContext
branch doesn't account for escaped parens, causing mismatched depth when
encountering \(' or \)'; update the loop (the code using input.next,
input.advance, OPEN_PAREN and CLOSE_PAREN) to detect and skip escaped characters
(e.g., when a backslash precedes a paren) so escaped '(' or ')' do not change
depth or terminate the loop, and ensure this behavior mirrors
hasBalancedParensAt's escape handling for consistency.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e3e0a6 and 3f7be64.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • packages/queryLanguage/src/tokens.ts
  • packages/queryLanguage/test/negation.txt
  • packages/queryLanguage/test/prefixes.txt

…orkflow

Replaces the two separate workflows with a single `test.yml` that runs
`yarn test` at the repo root, which executes all workspace tests
topologically via `yarn workspaces foreach`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@brendan-kellam brendan-kellam merged commit da26b90 into main Feb 26, 2026
6 of 7 checks passed
@brendan-kellam brendan-kellam deleted the brendan/fix-query-parser-paren-value branch February 26, 2026 20:12
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