[LFX-V2-199] Query Service: org search - suggestions endpoint#16
Conversation
- Added a new endpoint for organization suggestions based on a search query. - Integrated Clearbit's Autocomplete API for fetching organization suggestions. - Updated the service layer to handle the new suggestion logic and response formatting. - Enhanced the OpenAPI documentation to include the new endpoint and its parameters. - Updated CLI commands to support the new suggest-orgs functionality. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-199 Generated with [Cursor](https://cursor.com/) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an organization suggestions (typeahead) feature: API and DSL types, HTTP route and auth rule, service handler and converters, domain criteria/models and port, Clearbit autocomplete client/config/models, mock and service implementations, Helm chart bump, and minor HTTP client logging/retry tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant GW as Gateway (HTTPRoute/RuleSet)
participant QS as query-svc
participant SVC as OrganizationSearch (app)
participant SRCH as OrganizationSearcher (Clearbit)
participant CB as Clearbit Autocomplete API
C->>GW: GET /query/orgs/suggest?v=1&query=q\nAuthorization: Bearer ...
rect #EBF5FF
note over GW: OIDC auth (+optional contextualizer), allow-all, JWT finalizer
end
GW->>QS: Forward validated request
QS->>QS: payload → OrganizationSuggestionCriteria
QS->>SVC: SuggestOrganizations(criteria)
SVC->>SRCH: SuggestOrganizations(ctx, criteria)
SRCH->>CB: GET /autocomplete?query=q
CB-->>SRCH: 200 OK [suggestions]
SRCH-->>SVC: OrganizationSuggestionsResult
SVC-->>QS: OrganizationSuggestionsResult
QS->>QS: map domain model → API response
QS-->>GW: 200 OK { suggestions: [...] }
GW-->>C: 200 OK
sequenceDiagram
autonumber
participant QS as query-svc
participant SVC as OrganizationSearch
participant SRCH as Clearbit Searcher
participant CB as Clearbit Company API
note over QS,SRCH: Updated enrichment in QueryOrganizations
QS->>SVC: QueryOrganizations(criteria: name)
SVC->>SRCH: QueryOrganizations
SRCH->>CB: FindCompanyByName(name)
CB-->>SRCH: Company (may include domain)
alt company has domain
SRCH->>CB: FindCompanyByDomain(domain)
CB-->>SRCH: Enriched Company
SRCH->>SRCH: Replace with enriched result
end
SRCH-->>SVC: OrganizationResult or not-found error
SVC-->>QS: Result or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new organization suggestions feature for typeahead search functionality. It adds a new /query/orgs/suggest endpoint that provides organization suggestions based on a query string, extending the existing organization search capabilities.
- Adds new SuggestOrgs endpoint for typeahead organization search
- Extends Clearbit integration with Autocomplete API support
- Updates configuration to support separate autocomplete service base URL
Reviewed Changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/httpclient/client.go | Improves error handling and logging in HTTP client |
| internal/service/organization_search.go | Adds SuggestOrganizations business logic method |
| internal/infrastructure/mock/organization_searcher.go | Implements mock organization suggestions functionality |
| internal/infrastructure/clearbit/searcher.go | Adds Clearbit Autocomplete API integration and improves error handling |
| internal/infrastructure/clearbit/models.go | Defines ClearbitCompanySuggestion model |
| internal/infrastructure/clearbit/config.go | Adds autocomplete base URL configuration |
| internal/infrastructure/clearbit/client.go | Implements SuggestCompanies method and refactors makeRequest |
| internal/domain/port/searcher.go | Adds SuggestOrganizations interface method |
| internal/domain/model/*.go | Defines organization suggestion domain models |
| gen/* | Generated code for new suggest-orgs endpoint |
| design/*.go | API design definitions for organization suggestions |
| cmd/service/*.go | Service implementation and converters |
| charts/* | Deployment configuration updates |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/infrastructure/clearbit/client.go (1)
84-110: Critical: json.Unmarshal target is incorrect; results won’t populate.Passing &model (pointer to interface) prevents unmarshalling into the caller’s typed struct/slice; it will decode into a local interface instead. This yields zero-valued results and subtle bugs.
- if err := json.Unmarshal(resp.Body, &model); err != nil { + if err := json.Unmarshal(resp.Body, model); err != nil { return errors.NewUnexpected("failed to decode response", err) } return nilAlso consider refining HTTP error mapping:
- case http.StatusNotFound: - return errors.NewNotFound("company not found") + case http.StatusNotFound: + return errors.NewNotFound("resource not found") + case http.StatusUnauthorized, http.StatusForbidden: + return errors.NewUnexpected("authentication failed", err)Do you want a quick unit test to catch this regression (unmarshal into struct and slice)?
🧹 Nitpick comments (13)
charts/lfx-v2-query-service/templates/httproute.yaml (1)
16-20: PathPrefix may overmatch; consider adding Exact match for base pathPathPrefix "/query/orgs" also matches "/query/orgs-anything". If you want only the base and its subpaths, add an Exact match for "/query/orgs" and keep PathPrefix for "/query/orgs/*".
Example patch:
rules: - matches: - - path: - type: PathPrefix - value: /query/orgs + - path: + type: Exact + value: /query/orgs + - path: + type: PathPrefix + value: /query/orgs/ {{- if .Values.heimdall.enabled }} filters: - type: ExtensionRefAlso applies to: 28-34
internal/domain/model/search_criteria.go (1)
52-56: Add minimal validation/guards for suggestions queryConsider trimming and enforcing a small min/max length (e.g., 1–128) to protect upstream providers and avoid noisy calls on empty/long inputs. You can enforce at the converter layer if you prefer to keep the domain type simple.
internal/infrastructure/clearbit/models.go (1)
60-70: Logo field: optional encode-hintIf this type is ever marshaled (not just unmarshaled), add
omitemptyto align with domain JSON tags and avoid emitting nulls.Apply:
type ClearbitCompanySuggestion struct { // Name is the company name Name string `json:"name"` // Domain is the company's primary domain Domain string `json:"domain"` // Logo is the URL to the company's logo (can be null) - Logo *string `json:"logo"` + Logo *string `json:"logo,omitempty"` }pkg/httpclient/client.go (1)
137-138: Retry 408 Request Timeout as wellTreat 408 as transient alongside 5xx and 429 to improve resilience on flaky networks.
Apply:
- return retryableErr.StatusCode >= http.StatusInternalServerError || retryableErr.StatusCode == http.StatusTooManyRequests + return retryableErr.StatusCode >= http.StatusInternalServerError || + retryableErr.StatusCode == http.StatusTooManyRequests || + retryableErr.StatusCode == http.StatusRequestTimeoutdesign/types.go (1)
103-116: Validate domain (and consider URL format for logo).Add a domain pattern to keep response schema consistent with query-orgs; optionally add URL format for logo.
dsl.Attribute("domain", dsl.String, "Organization domain", func() { dsl.Example("linuxfoundation.org") + dsl.Pattern(`^[a-zA-Z0-9][a-zA-Z0-9-_.]*[a-zA-Z0-9]*\.[a-zA-Z]{2,}$`) })(Optional) Consider adding a URL format to logo.
internal/infrastructure/clearbit/config.go (1)
64-66: Align default timeout with DefaultConfig (30s).NewConfig defaults to 10s while DefaultConfig uses 30s. Use the same default to avoid surprises.
- if timeout == "" { - timeout = "10s" - } + if timeout == "" { + timeout = "30s" + }cmd/service/service.go (1)
100-109: Nil-safe conversion before building response.Guard against a nil domain result to prevent a panic in the converter.
result, errSuggestOrgs := s.organizationService.SuggestOrganizations(ctx, criteria) if errSuggestOrgs != nil { return nil, wrapError(ctx, errSuggestOrgs) } + if result == nil { + return &querysvc.SuggestOrgsResult{Suggestions: nil}, nil + } + // Convert domain result to response res = s.domainOrganizationSuggestionsToResponse(result) return res, nilinternal/infrastructure/mock/organization_searcher.go (1)
137-150: Short-circuit after reaching the cap (avoid scanning full list).Break once 5 suggestions are collected to keep mock behavior efficient with larger datasets.
for _, org := range m.organizations { if strings.Contains(strings.ToLower(org.Name), query) || strings.Contains(strings.ToLower(org.Domain), query) { suggestions = append(suggestions, model.OrganizationSuggestion{ Name: org.Name, Domain: org.Domain, Logo: nil, // Mock doesn't have logo data }) + if len(suggestions) >= 5 { + break + } } } - // Limit to first 5 suggestions for realistic behavior - if len(suggestions) > 5 { - suggestions = suggestions[:5] - } + // Already capped during iterationcmd/service/converters.go (1)
102-109: Normalize input query.Trim whitespace to avoid accidental empty/space-only queries reaching the domain layer.
func (s *querySvcsrvc) payloadToOrganizationSuggestionCriteria(ctx context.Context, p *querysvc.SuggestOrgsPayload) model.OrganizationSuggestionCriteria { criteria := model.OrganizationSuggestionCriteria{ - Query: p.Query, + Query: strings.TrimSpace(p.Query), } return criteria }Add import:
import "strings"internal/infrastructure/clearbit/searcher.go (3)
51-55: Clarify enrichment logging to avoid confusion.Log that enrichment happened and include the domain used; otherwise this looks identical to the earlier “found by domain” log.
- slog.DebugContext(ctx, "found organization by domain", "name", clearbitCompany.Name) + slog.DebugContext(ctx, "enriched organization via domain", + "domain", clearbitCompany.Domain, + "name_before", clearbitCompany.Name, + "name_after", clearbitCompanyEnriched.Name) clearbitCompany = clearbitCompanyEnriched
59-63: Avoid error-level log + returning the same error (double logging).Consider downgrading to Debug or removing the log to prevent duplicate error logs at call sites.
- slog.ErrorContext(ctx, "error searching organization", "error", err) + slog.DebugContext(ctx, "error searching organization", "error", err) return nil, err
64-67: Return validation error when no criteria provided (not “not found”).If both name and domain are nil, this is a bad request, not a not-found. Recommend explicit validation early.
Proposed guard near the start of the function (outside the changed hunk):
if criteria.Domain == nil && criteria.Name == nil { return nil, errors.NewValidation("either name or domain must be provided") }Would you like me to push a follow-up PR with this check and a unit test?
internal/infrastructure/clearbit/client.go (1)
63-82: Guard against missing Autocomplete base URL.If AutocompleteBaseURL is empty, url.Parse yields a relative URL and the request will fail at runtime. Fail fast.
func (c *Client) SuggestCompanies(ctx context.Context, query string) ([]ClearbitCompanySuggestion, error) { + if c.config.AutocompleteBaseURL == "" { + return nil, errors.NewUnexpected("autocomplete base URL not configured", nil) + } // Build the URL with query parameters u, err := url.Parse(fmt.Sprintf("%s/v1/companies/suggest", c.config.AutocompleteBaseURL))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (17)
gen/http/cli/lfx_v2_query_service/cli.gois excluded by!**/gen/**gen/http/openapi.jsonis excluded by!**/gen/**gen/http/openapi.yamlis excluded by!**/gen/**gen/http/openapi3.jsonis excluded by!**/gen/**gen/http/openapi3.yamlis excluded by!**/gen/**gen/http/query_svc/client/cli.gois excluded by!**/gen/**gen/http/query_svc/client/client.gois excluded by!**/gen/**gen/http/query_svc/client/encode_decode.gois excluded by!**/gen/**gen/http/query_svc/client/paths.gois excluded by!**/gen/**gen/http/query_svc/client/types.gois excluded by!**/gen/**gen/http/query_svc/server/encode_decode.gois excluded by!**/gen/**gen/http/query_svc/server/paths.gois excluded by!**/gen/**gen/http/query_svc/server/server.gois excluded by!**/gen/**gen/http/query_svc/server/types.gois excluded by!**/gen/**gen/query_svc/client.gois excluded by!**/gen/**gen/query_svc/endpoints.gois excluded by!**/gen/**gen/query_svc/service.gois excluded by!**/gen/**
📒 Files selected for processing (18)
charts/lfx-v2-query-service/Chart.yaml(1 hunks)charts/lfx-v2-query-service/templates/httproute.yaml(2 hunks)charts/lfx-v2-query-service/templates/ruleset.yaml(1 hunks)cmd/service/converters.go(1 hunks)cmd/service/providers.go(3 hunks)cmd/service/service.go(1 hunks)design/query-svc.go(1 hunks)design/types.go(1 hunks)internal/domain/model/organization.go(1 hunks)internal/domain/model/search_criteria.go(1 hunks)internal/domain/port/searcher.go(1 hunks)internal/infrastructure/clearbit/client.go(3 hunks)internal/infrastructure/clearbit/config.go(3 hunks)internal/infrastructure/clearbit/models.go(1 hunks)internal/infrastructure/clearbit/searcher.go(2 hunks)internal/infrastructure/mock/organization_searcher.go(1 hunks)internal/service/organization_search.go(2 hunks)pkg/httpclient/client.go(5 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
internal/domain/port/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place domain interfaces (ports) under internal/domain/port/
Files:
internal/domain/port/searcher.go
internal/domain/model/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core business entities (e.g., Resource, SearchCriteria, AccessCheck) under internal/domain/model/
Files:
internal/domain/model/search_criteria.gointernal/domain/model/organization.go
internal/infrastructure/mock/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain mock implementations under internal/infrastructure/mock/
Files:
internal/infrastructure/mock/organization_searcher.go
design/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep Goa API design specifications in the design/ directory
Files:
design/types.godesign/query-svc.go
internal/service/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep service-layer orchestration logic in internal/service/
Files:
internal/service/organization_search.go
🧠 Learnings (2)
📚 Learning: 2025-09-10T17:40:14.123Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-query-service#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T17:40:14.123Z
Learning: Applies to cmd/query_svc/query_svc.go : Keep the service implementation that connects Goa to domain logic in cmd/query_svc/query_svc.go
Applied to files:
cmd/service/converters.go
📚 Learning: 2025-09-08T18:22:24.173Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-query-service#13
File: charts/lfx-v2-query-service/templates/httproute.yaml:16-19
Timestamp: 2025-09-08T18:22:24.173Z
Learning: For the lfx-v2-query-service HTTPRoute configuration, OPTIONS method rules should not be included - only GET method constraints are needed for the /query/orgs endpoint.
Applied to files:
charts/lfx-v2-query-service/templates/httproute.yamlcharts/lfx-v2-query-service/templates/ruleset.yaml
🧬 Code graph analysis (10)
internal/domain/port/searcher.go (2)
internal/domain/model/search_criteria.go (1)
OrganizationSuggestionCriteria(53-56)internal/domain/model/organization.go (1)
OrganizationSuggestionsResult(31-34)
internal/infrastructure/mock/organization_searcher.go (2)
internal/domain/model/search_criteria.go (1)
OrganizationSuggestionCriteria(53-56)internal/domain/model/organization.go (2)
OrganizationSuggestionsResult(31-34)OrganizationSuggestion(21-28)
cmd/service/service.go (1)
gen/query_svc/service.go (2)
SuggestOrgsPayload(155-162)SuggestOrgsResult(166-169)
cmd/service/converters.go (3)
gen/query_svc/service.go (3)
SuggestOrgsPayload(155-162)SuggestOrgsResult(166-169)OrganizationSuggestion(84-91)internal/domain/model/search_criteria.go (1)
OrganizationSuggestionCriteria(53-56)internal/domain/model/organization.go (2)
OrganizationSuggestionsResult(31-34)OrganizationSuggestion(21-28)
design/query-svc.go (3)
design/types.go (1)
OrganizationSuggestion(103-116)internal/domain/model/organization.go (1)
OrganizationSuggestion(21-28)gen/query_svc/service.go (1)
OrganizationSuggestion(84-91)
internal/service/organization_search.go (2)
internal/domain/model/search_criteria.go (1)
OrganizationSuggestionCriteria(53-56)internal/domain/model/organization.go (1)
OrganizationSuggestionsResult(31-34)
internal/infrastructure/clearbit/client.go (3)
internal/infrastructure/clearbit/models.go (2)
ClearbitCompany(7-28)ClearbitCompanySuggestion(61-70)pkg/errors/client.go (2)
NewNotFound(39-46)NewValidation(19-26)pkg/errors/server.go (1)
NewUnexpected(19-26)
internal/infrastructure/clearbit/searcher.go (4)
pkg/errors/client.go (1)
NewNotFound(39-46)internal/domain/port/searcher.go (1)
OrganizationSearcher(26-35)internal/domain/model/search_criteria.go (1)
OrganizationSuggestionCriteria(53-56)internal/domain/model/organization.go (2)
OrganizationSuggestionsResult(31-34)OrganizationSuggestion(21-28)
internal/domain/model/organization.go (2)
design/types.go (1)
OrganizationSuggestion(103-116)gen/query_svc/service.go (1)
OrganizationSuggestion(84-91)
internal/infrastructure/clearbit/config.go (1)
pkg/httpclient/config.go (1)
Config(11-23)
🔇 Additional comments (10)
charts/lfx-v2-query-service/Chart.yaml (1)
8-8: Chart version bump looks good0.4.1 aligns with the new org-suggest surface; no concerns.
charts/lfx-v2-query-service/templates/ruleset.yaml (1)
64-79: New org-suggest rule is consistent with existing auth/finalizer patternGET-only, OIDC, optional contextualizer, allow_all, and aud set correctly. Matches the prior “no OPTIONS” learning.
internal/domain/model/organization.go (1)
20-34: Domain models for suggestions are well-scopedTypes map cleanly to API and infrastructure. No issues.
internal/domain/port/searcher.go (1)
30-32: SuggestOrganizations added — implementations updatedinternal/domain/port/searcher.go contains the interface; matching implementations found in internal/infrastructure/clearbit/searcher.go, internal/infrastructure/mock/organization_searcher.go, and internal/service/organization_search.go.
cmd/service/providers.go (1)
165-200: Plumb CLEARBIT_AUTOCOMPLETE_BASE_URL through deployment config.
cmd/service/providers.go (lines 165–200) reads CLEARBIT_AUTOCOMPLETE_BASE_URL but a search returned no matches under charts/ — ensure Helm values.yaml and deployment templates (or other deployment manifests) export this env var to the service container.design/query-svc.go (1)
130-167: LGTM — endpoint design is clear and consistent.internal/service/organization_search.go (2)
65-92: LGTM — thin delegation with helpful telemetry.
21-23: Resolved — OrganizationSearcher implementers updated.
Search confirms SuggestOrganizations is declared in internal/domain/port/searcher.go and implemented in internal/infrastructure/clearbit/searcher.go and internal/infrastructure/mock/organization_searcher.go; internal/service/organization_search.go calls it.internal/infrastructure/clearbit/client.go (2)
35-41: Call site looks fine once makeRequest is fixed.No additional concerns here; the local model allocation pattern is correct.
55-61: Same as above; OK once makeRequest is fixed.The domain path and query param assembly are fine.
- Added a check in the domainOrganizationSuggestionsToResponse function to return an empty suggestions result when the input is nil or has no suggestions. - This enhancement improves the robustness of the response handling for organization suggestions. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-199 Reviewed with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
Overview
This pull request introduces a new "organization suggestions" feature for typeahead search in the query service, along with supporting updates across the API, service implementation, CLI, and deployment configuration. The main changes add a new endpoint for organization suggestions, update the data model and CLI to support it, and enhance configuration for external autocomplete services.
Note: There’s a specific ticket to improve unit tests (and maybe integration tests).
Test Results Summary
1.1
/query/orgs/suggest?v=1&query=magazine123✅1.2
/query/orgs/suggest?v=1&query=linux✅Organizations Found:
2.
/query/orgs?v=1&name=magazine123✅3.
/query/orgs?v=1&domain=linuxfoundation.org✅Key Observations