language: Increase cursor match limit - #36802
Conversation
cla-bot
Bot
added
the
cla-signed
maxdeviant
changed the title
|
Note: I just wrote a little script to run the table test query on all test files in one of my repos with a total of 276 test files, to validate various limits and see whether any file is exceeding it:
|
|
@Spissable @osiewicz I think the problem is that we're creating a separate query match for every entry in the test table, and all of those query matches overlap with each other, because they match a specific element of the table and the subsequent for loop. So as the query is being matched, we need to maintain a separate in-progress match for every entry in the table. What if instead, we used a repetition in the query, so that we could produce a single match that captured all of the test case names (by capturing multiple nodes with the the We would need to make some changes to the One option would be to allow multiple matches for any |
maxbrunsfeld
left a comment
maxbrunsfeld
left a comment
There was a problem hiding this comment.
I want us to try other solutions before bumping this, for performance reasons.
|
Agreed that bumping the match count is not ideal here.
This worries me a bit; All of that is to say that I think that code that produces |
|
Thanks for the feedback, I'll try to find some time this week and see what I can do |
|
Another option is to post-process the query matches, so that if we capture multiple nodes with the same capture, (in this case, ‘_table_test_case_name’), we create multiple TaskVariables struct, almost as if we had gotten multiple distinct matches. Not sure if that is more intuitive than allowing multiple values for a variable. |
- reduce cursor match limit to 128 - split slice and map into separate queries
5a085f0 to
9334a33
Compare
|
Hey @maxbrunsfeld @osiewicz - I've been hitting a wall trying to make your suggestions work. So I'd like to propose the following change (see last commit): @osiewicz asked me in the original PR to combine the slice and map version of the query into one. This was before I was aware of the tight cursor match limit. Since the alternations are contributing quite a bit to the match limit issue, what's your opinions on having 2 queries again? In comparison, with this approach I reduced the amount of exceeding test files in my main project from 45 to 3. I'd still like to propose to at least bump the limit to 128. For my workflow this number seems to be great. In case you insist that it shall remain at 64, I just have reduce the slice test case from 9 to 8 sub-tests, otherwise it's gonna fail. Btw. I also found #22042 that also revolved around the match limit. |
|
A slight status update: |
github-project-automation
Bot
moved this to Community PRs
in Quality Week – December 2025
|
Ok, for now I'm going to close this out, because it's pretty important that we place a strict bound on how much work the syntactic queries can do during matching. I still think there are multiple good options for how we can improve our runnables query logic to enable fixing this in an efficient way. |
github-project-automation
Bot
moved this from Community PRs
to Done
in Quality Week – December 2025
…er match (#57276) Sets up infra for fixing #46881. Follow-up to the approach discussed in #36802 (comment). The initial problem: Go table tests weren't being detected reliably in larger files. The old approach created one tree-sitter match per table row, which hit the cursor match limit (64) and caused rows to be silently dropped. The suggested fix was to use query repetition to capture all rows in a single match, then post-process in Rust. But this hit another problem: some captured rows aren't valid runnables (e.g., when a struct has multiple string fields and the query can't tell which one is the test name). Tree-sitter predicates can't validate subsets of captures within a match, that logic needs to happen in Rust. This PR adds a `RunnableResolver` trait that lets languages post-process multi-capture matches. When a query uses `@_run_item` to mark item boundaries, we split the captures into per-item groups and pass them to the resolver. The resolver can then: - Pick which `@run` to use: e.g., compare field names against `t.Run(tc.name, ...)` to find the right string field. - Return per-item extras: only the captures that correspond to the chosen `@run`. Release Notes: - N/A
…er match (zed-industries#57276) Sets up infra for fixing zed-industries#46881. Follow-up to the approach discussed in zed-industries#36802 (comment). The initial problem: Go table tests weren't being detected reliably in larger files. The old approach created one tree-sitter match per table row, which hit the cursor match limit (64) and caused rows to be silently dropped. The suggested fix was to use query repetition to capture all rows in a single match, then post-process in Rust. But this hit another problem: some captured rows aren't valid runnables (e.g., when a struct has multiple string fields and the query can't tell which one is the test name). Tree-sitter predicates can't validate subsets of captures within a match, that logic needs to happen in Rust. This PR adds a `RunnableResolver` trait that lets languages post-process multi-capture matches. When a query uses `@_run_item` to mark item boundaries, we split the captures into per-item groups and pass them to the resolver. The resolver can then: - Pick which `@run` to use: e.g., compare field names against `t.Run(tc.name, ...)` to find the right string field. - Return per-item extras: only the captures that correspond to the chosen `@run`. Release Notes: - N/A
…er match (zed-industries#57276) Sets up infra for fixing zed-industries#46881. Follow-up to the approach discussed in zed-industries#36802 (comment). The initial problem: Go table tests weren't being detected reliably in larger files. The old approach created one tree-sitter match per table row, which hit the cursor match limit (64) and caused rows to be silently dropped. The suggested fix was to use query repetition to capture all rows in a single match, then post-process in Rust. But this hit another problem: some captured rows aren't valid runnables (e.g., when a struct has multiple string fields and the query can't tell which one is the test name). Tree-sitter predicates can't validate subsets of captures within a match, that logic needs to happen in Rust. This PR adds a `RunnableResolver` trait that lets languages post-process multi-capture matches. When a query uses `@_run_item` to mark item boundaries, we split the captures into per-item groups and pass them to the resolver. The resolver can then: - Pick which `@run` to use: e.g., compare field names against `t.Run(tc.name, ...)` to find the right string field. - Return per-item extras: only the captures that correspond to the chosen `@run`. Release Notes: - N/A
…er match (zed-industries#57276) Sets up infra for fixing zed-industries#46881. Follow-up to the approach discussed in zed-industries#36802 (comment). The initial problem: Go table tests weren't being detected reliably in larger files. The old approach created one tree-sitter match per table row, which hit the cursor match limit (64) and caused rows to be silently dropped. The suggested fix was to use query repetition to capture all rows in a single match, then post-process in Rust. But this hit another problem: some captured rows aren't valid runnables (e.g., when a struct has multiple string fields and the query can't tell which one is the test name). Tree-sitter predicates can't validate subsets of captures within a match, that logic needs to happen in Rust. This PR adds a `RunnableResolver` trait that lets languages post-process multi-capture matches. When a query uses `@_run_item` to mark item boundaries, we split the captures into per-item groups and pass them to the resolver. The resolver can then: - Pick which `@run` to use: e.g., compare field names against `t.Run(tc.name, ...)` to find the right string field. - Return per-item extras: only the captures that correspond to the chosen `@run`. Release Notes: - N/A
…er match (zed-industries#57276) Sets up infra for fixing zed-industries#46881. Follow-up to the approach discussed in zed-industries#36802 (comment). The initial problem: Go table tests weren't being detected reliably in larger files. The old approach created one tree-sitter match per table row, which hit the cursor match limit (64) and caused rows to be silently dropped. The suggested fix was to use query repetition to capture all rows in a single match, then post-process in Rust. But this hit another problem: some captured rows aren't valid runnables (e.g., when a struct has multiple string fields and the query can't tell which one is the test name). Tree-sitter predicates can't validate subsets of captures within a match, that logic needs to happen in Rust. This PR adds a `RunnableResolver` trait that lets languages post-process multi-capture matches. When a query uses `@_run_item` to mark item boundaries, we split the captures into per-item groups and pass them to the resolver. The resolver can then: - Pick which `@run` to use: e.g., compare field names against `t.Run(tc.name, ...)` to find the right string field. - Return per-item extras: only the captures that correspond to the chosen `@run`. Release Notes: - N/A
5 participants
With my last PR adding support to run individual Go table sub-tests, I and some other people noticed that it hasn't been working quite as expected, especially for larger table tests. Basically in larger table tests, the detection would only work bottom up, up to a certain point.
After debugging for a while and also confirming that the query seems to work just fine (via tree-sitter playground) I realized that the cursor limit is causing it to only find a limited amount of sub-tests.
I made my test example a bit longer to provoke a failure and then bumped the limit to 256 fixed it. Trying it out with some of my real world tests also yielded correct results.
I'm not sure what the golden number could possibly be, but 64 was definitely too low for my day to day use-cases.
If we're worried about performance, I suppose we could also play with
ts_query_cursor_set_timeout_microsrather than capping the cursor limit at a fixed number.Release Notes: