Skip to content
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

insights: Add capture group aggregation #40714

Merged
merged 21 commits into from
Aug 25, 2022
Merged

Conversation

CristinaBirkel
Copy link
Contributor

@CristinaBirkel CristinaBirkel commented Aug 23, 2022

Closes #40211

Description

This adds in capture group aggregation. I copied over some functionality primarily from the compute package, but made a couple small modifications. I don't think we want to take an actual dependency on compute for this in any case.

Let me know if you can think of anything else we need to add here for now--I think it's in a pretty good state though to start testing and make sure it integrates with FE, and we can plan to iterate on edge cases?

Test plan

So far, testing locally, I get a lot of timeouts. But here's a query that worked for me:

{
  searchQueryAggregate(query:"query(s.*)$", patternType: regexp) {
    aggregations(mode: CAPTURE_GROUP, limit: 10) {
      __typename
      ... on ExhaustiveSearchAggregationResult {
        groups {
          label
          count
          query
        }
        otherResultCount
        otherGroupCount
      }
      ... on SearchAggregationNotAvailable {
        reason
      }
    }
  }
}

How else should we be testing this? It looks as expected to me, but let me know what you think!

@cla-bot cla-bot bot added the cla-signed label Aug 23, 2022
@CristinaBirkel CristinaBirkel marked this pull request as draft August 23, 2022 03:47
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Aug 23, 2022

Codenotify: Notifying subscribers in CODENOTIFY files for diff a2b623d...d5c6a73.

Notify File(s)
@sourcegraph/code-insights-backend enterprise/internal/insights/aggregation/aggregation.go
enterprise/internal/insights/aggregation/aggregation_test.go
enterprise/internal/insights/aggregation/capture_group_helpers.go
enterprise/internal/insights/query/querybuilder/regexp.go
enterprise/internal/insights/resolvers/aggregates_resolvers.go
enterprise/internal/insights/resolvers/aggregates_resolvers_test.go

@CristinaBirkel CristinaBirkel marked this pull request as ready for review August 24, 2022 16:41
@CristinaBirkel CristinaBirkel merged commit a638173 into main Aug 25, 2022
@CristinaBirkel CristinaBirkel deleted the insights/add-capture-groups branch August 25, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

insights: extend insights aggregation search decoder to support capture group matches
5 participants