diff --git a/.env.example b/.env.example index d7068713..d48d0c1d 100644 --- a/.env.example +++ b/.env.example @@ -108,6 +108,24 @@ WEBHOOK_MAX_COUNT=500 # GOOGLE_APPLICATION_CREDENTIALS= (optional; for google-gemini when outside Google Cloud: path to service account key JSON) # EMBEDDING_MODEL= (required to enable embeddings; no default) +# Translation (language enrichment) is optional. To enable, set both TRANSLATION_PROVIDER and TRANSLATION_MODEL; if either is unset, translation is disabled and no translation jobs run. +# Open-text feedback (value_text) is translated into each tenant's configured target_language (Hub tenant settings). Same providers/auth model as embeddings. +# Configure identically in the API and worker processes (the API enqueues, the worker translates); otherwise jobs pile up unprocessed. +# TRANSLATION_PROVIDER=openai +# TRANSLATION_PROVIDER=google-gemini +# TRANSLATION_PROVIDER_API_KEY=sk-... (required for openai and google; not used for google-gemini) +# TRANSLATION_BASE_URL=https://llm.example.com/v1 (optional; only supported with TRANSLATION_PROVIDER=openai) +# TRANSLATION_GOOGLE_CLOUD_PROJECT= (required for google-gemini; or use GOOGLE_CLOUD_PROJECT) +# TRANSLATION_GOOGLE_CLOUD_LOCATION= (required for google-gemini, e.g. europe-west3; or use GOOGLE_CLOUD_LOCATION) +# TRANSLATION_MODEL= (required to enable translation; no default; e.g. gemini-2.5-flash) +# TRANSLATION_MAX_CONCURRENT=5 (worker concurrency; default 5) +# TRANSLATION_MAX_ATTEMPTS=3 (River job retries before failing; default 3) + +# Tenant settings cache (optional): an in-process LRU+TTL over tenant target_language reads on the translation enqueue path. +# Set TENANT_SETTINGS_CACHE_SIZE=0 to disable the cache. Defaults: size 2048, TTL 60s. +# TENANT_SETTINGS_CACHE_SIZE=2048 +# TENANT_SETTINGS_CACHE_TTL_SECONDS=60 + # Tenant data purge (optional). How long DELETE /v1/tenants/{tenant_id}/data waits for in-flight # tenant-owned writes to drain before returning a retryable 409 (code tenant_write_conflict). # Must be a positive integer (seconds); non-positive values fall back to the default. Default: 5 diff --git a/cmd/api/app.go b/cmd/api/app.go index 6878f39f..b7c7f10d 100644 --- a/cmd/api/app.go +++ b/cmd/api/app.go @@ -174,14 +174,16 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { } var ( - eventMetrics observability.EventMetrics - webhookMetrics observability.WebhookMetrics - embeddingMetrics observability.EmbeddingMetrics + eventMetrics observability.EventMetrics + webhookMetrics observability.WebhookMetrics + embeddingMetrics observability.EmbeddingMetrics + translationMetrics observability.TranslationMetrics ) if metrics != nil { eventMetrics = metrics.Events webhookMetrics = metrics.Webhooks embeddingMetrics = metrics.Embeddings + translationMetrics = metrics.Translation } var tracerProvider *sdktrace.TracerProvider @@ -277,6 +279,40 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { searchHandler = handlers.NewSearchHandler(nil) // 503 when embeddings disabled } + // Register the translation worker and declare its queue so the River client can + // enqueue translation jobs (River requires the job kind registered and the queue + // declared at insert time); the jobs are processed by hub-worker, not in this + // process. Gated on TRANSLATION_PROVIDER+MODEL like embeddings; the enqueue + // provider is registered below, after the River client and tenant settings exist. + if cfg.Translation.Provider != "" && cfg.Translation.Model != "" { + translationCfg := service.TranslationClientConfig{ + Provider: cfg.Translation.Provider, + ProviderAPIKey: cfg.Translation.ProviderAPIKey, + Model: cfg.Translation.Model, + BaseURL: cfg.Translation.BaseURL, + GoogleCloudProject: cfg.Translation.GoogleCloudProject, + GoogleCloudLocation: cfg.Translation.GoogleCloudLocation, + } + + translationClient, translationErr := service.NewTranslationClient(context.Background(), translationCfg) + if translationErr != nil { + cleanupNewAppStartupFailure(context.Background(), messageManager, nil, tracerProvider, meterProvider) + + return nil, fmt.Errorf("translation config: %w", translationErr) + } + + river.AddWorker(riverWorkers, workers.NewFeedbackTranslationWorker(feedbackRecordsService, translationClient, translationMetrics)) + + queues[service.TranslationsQueueName] = river.QueueConfig{MaxWorkers: 1} + + // Per-tenant re-translation backfill, enqueued by the settings-change listener + // below. Registered here only so the River client can validate the kind and queue + // at insert time; the fan-out is processed by hub-worker. + river.AddWorker(riverWorkers, workers.NewTenantTranslationBackfillWorker(feedbackRecordsService, cfg.Translation.MaxAttempts)) + + queues[service.TranslationBackfillsQueueName] = river.QueueConfig{MaxWorkers: 1} + } + riverClient, err := river.NewClient(riverpgxv5.New(db), &river.Config{ Queues: queues, Workers: riverWorkers, @@ -325,6 +361,29 @@ func NewApp(cfg *config.Config, db *pgxpool.Pool) (*App, error) { tenantSettingsService := service.NewTenantSettingsService(tenantSettingsRepo) tenantSettingsHandler := handlers.NewTenantSettingsHandler(tenantSettingsService) + // Translation enqueue provider: on a feedback-record create/update it resolves the + // tenant's target language (through a short-TTL cache over tenant settings) and + // enqueues a translation job. Gated on TRANSLATION_PROVIDER+MODEL. + if cfg.Translation.Provider != "" && cfg.Translation.Model != "" { + var translationCacheMetrics observability.CacheMetrics + if metrics != nil { + translationCacheMetrics = metrics.Cache + } + + translationCache := service.NewCachedTenantSettings( + tenantSettingsService, + cfg.TenantSettingsCache.Size, cfg.TenantSettingsCache.TTL.Duration(), + translationCacheMetrics, + ) + messageManager.RegisterProvider(service.NewTranslationProvider( + riverClient, translationCache, service.TranslationsQueueName, cfg.Translation.MaxAttempts, translationMetrics)) + + // On a target_language change, enqueue a per-tenant re-translation backfill so + // existing records pick up the new target (not only newly ingested ones). + tenantSettingsService.SetSettingsChangeListener(service.NewTranslationSettingsListener( + riverClient, service.TranslationBackfillsQueueName, cfg.Translation.MaxAttempts)) + } + taxonomyRepo := repository.NewTaxonomyRepository(db) var taxonomyStarter service.TaxonomyRunStarter diff --git a/cmd/backfill-translations/main.go b/cmd/backfill-translations/main.go new file mode 100644 index 00000000..371524d9 --- /dev/null +++ b/cmd/backfill-translations/main.go @@ -0,0 +1,131 @@ +// backfill-translations enqueues River translation jobs for feedback records whose +// tenant has a target language configured and whose value_text is not yet translated +// to it (missing or stale). Run this one-off after enabling translation or changing a +// tenant's target language; hub-worker (or the API process) runs the jobs. +package main + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + + "github.com/riverqueue/river" + "github.com/riverqueue/river/riverdriver/riverpgxv5" + + "github.com/formbricks/hub/internal/config" + "github.com/formbricks/hub/internal/repository" + "github.com/formbricks/hub/internal/service" + "github.com/formbricks/hub/internal/workers" + "github.com/formbricks/hub/pkg/database" +) + +var ( + errTranslationProviderRequired = errors.New("TRANSLATION_PROVIDER is required") + errTranslationModelRequired = errors.New("TRANSLATION_MODEL is required") +) + +const ( + defaultTranslationMaxAttempts = 3 + exitSuccess = 0 + exitFailure = 1 +) + +func main() { + os.Exit(run()) +} + +func run() int { + cfg, err := config.Load() + if err != nil { + slog.Error("Failed to load configuration", "error", err) + + return exitFailure + } + + if cfg.Database.URL == "" || cfg.Database.URL == config.DefaultDatabaseURL { + slog.Error("DATABASE_URL must be set explicitly for this binary (do not use the default test URL)") + + return exitFailure + } + + if cfg.Translation.Provider == "" { + slog.Error(errTranslationProviderRequired.Error()) + + return exitFailure + } + + if cfg.Translation.Model == "" { + slog.Error(errTranslationModelRequired.Error()) + + return exitFailure + } + + maxAttempts := cfg.Translation.MaxAttempts + if maxAttempts <= 0 { + maxAttempts = defaultTranslationMaxAttempts + } + + ctx := context.Background() + + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, + database.WithPoolConfig(cfg.Database.PoolConfig()), + ) + if err != nil { + slog.Error("Failed to connect to database", "error", err) + + return exitFailure + } + defer db.Close() + + translationCfg := service.TranslationClientConfig{ + Provider: cfg.Translation.Provider, + ProviderAPIKey: cfg.Translation.ProviderAPIKey, + Model: cfg.Translation.Model, + BaseURL: cfg.Translation.BaseURL, + GoogleCloudProject: cfg.Translation.GoogleCloudProject, + GoogleCloudLocation: cfg.Translation.GoogleCloudLocation, + } + + translationClient, err := service.NewTranslationClient(ctx, translationCfg) + if err != nil { + slog.Error("Failed to create translation client", "error", err) + + return exitFailure + } + + repo := repository.NewFeedbackRecordsRepository(db) + feedbackRecordsService := service.NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + // Producer-only: we only enqueue jobs; workers run in hub-worker (or the API process). + // River requires the job kind registered (worker added) and MaxWorkers > 0 for a declared queue. + riverWorkers := river.NewWorkers() + river.AddWorker(riverWorkers, workers.NewFeedbackTranslationWorker(feedbackRecordsService, translationClient, nil)) + + riverClient, err := river.NewClient(riverpgxv5.New(db), &river.Config{ + Queues: map[string]river.QueueConfig{ + service.TranslationsQueueName: {MaxWorkers: 1}, + }, + Workers: riverWorkers, + }) + if err != nil { + slog.Error("Failed to create River client", "error", err) + + return exitFailure + } + + enqueued, err := feedbackRecordsService.BackfillTranslations( + ctx, riverClient, service.TranslationsQueueName, maxAttempts) + if err != nil { + slog.Error("Backfill failed", "error", err) + + return exitFailure + } + + slog.Info("Backfill complete", "enqueued", enqueued) + + fmt.Printf("Enqueued %d translation job(s).\n", enqueued) + + return exitSuccess +} diff --git a/cmd/worker/app.go b/cmd/worker/app.go index 8fd50699..14273d91 100644 --- a/cmd/worker/app.go +++ b/cmd/worker/app.go @@ -68,13 +68,15 @@ func NewWorkerApp(cfg *config.Config, db *pgxpool.Pool) (*WorkerApp, error) { webhooksRepo := repository.NewWebhooksRepository(db) var ( - webhookMetrics observability.WebhookMetrics - embeddingMetrics observability.EmbeddingMetrics + webhookMetrics observability.WebhookMetrics + embeddingMetrics observability.EmbeddingMetrics + translationMetrics observability.TranslationMetrics ) if metrics != nil { webhookMetrics = metrics.Webhooks embeddingMetrics = metrics.Embeddings + translationMetrics = metrics.Translation } webhookSender := service.NewWebhookSenderImpl( @@ -130,6 +132,36 @@ func NewWorkerApp(cfg *config.Config, db *pgxpool.Pool) (*WorkerApp, error) { deps.EmbeddingMetrics = embeddingMetrics } + if cfg.Translation.Provider != "" && cfg.Translation.Model != "" { + translationCfg := service.TranslationClientConfig{ + Provider: cfg.Translation.Provider, + ProviderAPIKey: cfg.Translation.ProviderAPIKey, + Model: cfg.Translation.Model, + BaseURL: cfg.Translation.BaseURL, + GoogleCloudProject: cfg.Translation.GoogleCloudProject, + GoogleCloudLocation: cfg.Translation.GoogleCloudLocation, + } + + translationClient, err := service.NewTranslationClient(context.Background(), translationCfg) + if err != nil { + shutdownObservability(context.Background(), meterProvider, tracerProvider) + + return nil, fmt.Errorf("translation config: %w", err) + } + + // The translation worker only reads the record and writes the translation, so + // the embedding-specific service params are unused here. + translationRecordsRepo := repository.NewFeedbackRecordsRepository(db) + translationRecordsService := service.NewFeedbackRecordsService( + translationRecordsRepo, nil, "", nil, nil, "", 0) + + deps.TranslationService = translationRecordsService + deps.TranslationClient = translationClient + deps.TranslationMetrics = translationMetrics + deps.TranslationBackfillService = translationRecordsService + deps.TranslationMaxAttempts = cfg.Translation.MaxAttempts + } + riverWorkers, queues := workers.NewRiverWorkersAndQueues(cfg, deps, 0) riverCfg := &river.Config{ diff --git a/internal/config/config.go b/internal/config/config.go index 54e287a5..e9ee18be 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,6 +28,7 @@ var ( ErrDatabaseMinConnsExceedsMax = errors.New("DATABASE_MIN_CONNS must not exceed DATABASE_MAX_CONNS") ErrInvalidPublicBaseURL = errors.New("PUBLIC_BASE_URL must be an absolute http(s) URL without query or fragment") ErrInvalidEmbeddingBaseURL = errors.New("EMBEDDING_BASE_URL must be an absolute http(s) URL without query or fragment") + ErrInvalidTranslationBaseURL = errors.New("TRANSLATION_BASE_URL must be an absolute http(s) URL without query or fragment") ErrInvalidTaxonomyServiceURL = errors.New("TAXONOMY_SERVICE_URL must be an absolute http(s) URL without query or fragment") ) @@ -39,15 +40,17 @@ const DefaultDatabaseURL = "postgres://postgres:postgres@localhost:5432/test_db? // Config holds all application configuration in nested groups. type Config struct { - Server ServerConfig - Database DatabaseConfig - River RiverConfig - Webhook WebhookConfig - MessagePublisher MessagePublisherConfig - Embedding EmbeddingConfig - Taxonomy TaxonomyConfig - TenantData TenantDataConfig - Observability ObservabilityConfig + Server ServerConfig + Database DatabaseConfig + River RiverConfig + Webhook WebhookConfig + MessagePublisher MessagePublisherConfig + Embedding EmbeddingConfig + Translation TranslationConfig + TenantSettingsCache TenantSettingsCacheConfig + Taxonomy TaxonomyConfig + TenantData TenantDataConfig + Observability ObservabilityConfig } // ServerConfig holds HTTP server and process settings. @@ -127,6 +130,29 @@ type EmbeddingConfig struct { GoogleCloudLocation string `env:"EMBEDDING_GOOGLE_CLOUD_LOCATION"` } +// TranslationConfig holds the feedback open-text translation enrichment settings +// (ENG-1255). Translation is disabled unless Provider and Model are both set. +type TranslationConfig struct { + ProviderAPIKey string `env:"TRANSLATION_PROVIDER_API_KEY"` + Provider string `env:"TRANSLATION_PROVIDER"` + Model string `env:"TRANSLATION_MODEL"` + BaseURL string `env:"TRANSLATION_BASE_URL"` + MaxConcurrent int `env:"TRANSLATION_MAX_CONCURRENT" env-default:"5"` + MaxAttempts int `env:"TRANSLATION_MAX_ATTEMPTS" env-default:"3"` + GoogleCloudProject string `env:"TRANSLATION_GOOGLE_CLOUD_PROJECT"` + GoogleCloudLocation string `env:"TRANSLATION_GOOGLE_CLOUD_LOCATION"` +} + +// TenantSettingsCacheConfig configures the per-process tenant-settings cache that +// the translation enqueue gate and worker use to resolve a tenant's target +// language without hitting the database on every feedback event. A short TTL +// bounds cross-replica staleness; the worker records the target it actually used, +// so a changed target self-corrects on the next write. +type TenantSettingsCacheConfig struct { + Size int `env:"TENANT_SETTINGS_CACHE_SIZE" env-default:"2048"` + TTL DurationSec `env:"TENANT_SETTINGS_CACHE_TTL_SECONDS" env-default:"60"` +} + // TaxonomyConfig holds Hub-to-taxonomy service settings. type TaxonomyConfig struct { ServiceURL string `env:"TAXONOMY_SERVICE_URL"` @@ -289,6 +315,31 @@ func applyDefaults(cfg *Config) { cfg.Embedding.MaxAttempts = 3 } + if cfg.Translation.MaxConcurrent <= 0 { + cfg.Translation.MaxConcurrent = 5 + } + + if cfg.Translation.MaxAttempts <= 0 { + cfg.Translation.MaxAttempts = 3 + } + + // Default the cache size only when the operator did not set it. An explicit 0 (or + // negative) disables the cache: NewCachedTenantSettings treats size <= 0 as "no + // caching". cleanenv does not reliably apply env-default to nested-struct fields, so + // we cannot distinguish "unset" from an explicit "0" without consulting the env. + const defaultTenantSettingsCacheSize = 2048 + if _, ok := os.LookupEnv("TENANT_SETTINGS_CACHE_SIZE"); !ok { + cfg.TenantSettingsCache.Size = defaultTenantSettingsCacheSize + } + + // Mirror the other nested DurationSec defaults: re-assert in applyDefaults because + // cleanenv does not reliably apply env-default to nested-struct fields, and a zero TTL + // would silently disable the cache rather than use 60s. + const defaultTenantSettingsCacheTTLSec = 60 + if cfg.TenantSettingsCache.TTL.Duration() <= 0 { + cfg.TenantSettingsCache.TTL = DurationSec(time.Duration(defaultTenantSettingsCacheTTLSec) * time.Second) + } + if cfg.Taxonomy.MinimumEmbeddedRecords <= 0 { cfg.Taxonomy.MinimumEmbeddedRecords = 20 } @@ -359,6 +410,15 @@ func validate(cfg *Config) error { cfg.Embedding.BaseURL = normalized } + if cfg.Translation.BaseURL != "" { + normalized, err := normalizeHTTPBaseURL(cfg.Translation.BaseURL, ErrInvalidTranslationBaseURL) + if err != nil { + return err + } + + cfg.Translation.BaseURL = normalized + } + if cfg.Taxonomy.ServiceURL != "" { normalized, err := normalizeHTTPBaseURL(cfg.Taxonomy.ServiceURL, ErrInvalidTaxonomyServiceURL) if err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ba5cee63..fe699e19 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -194,6 +194,36 @@ func TestLoad_WebhookDeliveryMaxAttempts(t *testing.T) { }) } +func TestLoad_TenantSettingsCacheSize(t *testing.T) { + t.Run("explicit 0 disables and is not reset to the default", func(t *testing.T) { + t.Setenv("API_KEY", "test-api-key") + t.Setenv("TENANT_SETTINGS_CACHE_SIZE", "0") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + if cfg.TenantSettingsCache.Size != 0 { + t.Fatalf("Size = %d, want 0 (explicit 0 disables the cache, must not default to 2048)", + cfg.TenantSettingsCache.Size) + } + }) + + t.Run("unset applies the default", func(t *testing.T) { + t.Setenv("API_KEY", "test-api-key") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + if cfg.TenantSettingsCache.Size != 2048 { + t.Fatalf("Size = %d, want 2048 (default when unset)", cfg.TenantSettingsCache.Size) + } + }) +} + func TestLoad_EmbeddingGoogleCloudProject(t *testing.T) { t.Setenv("API_KEY", "test-api-key") t.Setenv("EMBEDDING_GOOGLE_CLOUD_PROJECT", "my-google-cloud-project") diff --git a/internal/googleai/client.go b/internal/googleai/client.go index 4419e372..c729b9b0 100644 --- a/internal/googleai/client.go +++ b/internal/googleai/client.go @@ -1,4 +1,5 @@ -// Package googleai provides a thin wrapper around the Google Gen AI SDK for embeddings. +// Package googleai provides a thin wrapper around the Google Gen AI SDK for +// embeddings and content generation (used for translation). package googleai import ( @@ -6,10 +7,13 @@ import ( "errors" "fmt" "math" + "net/http" "strings" + "time" "google.golang.org/genai" + "github.com/formbricks/hub/internal/huberrors" "github.com/formbricks/hub/internal/models" "github.com/formbricks/hub/pkg/embeddings" ) @@ -27,6 +31,8 @@ var ( ErrGoogleGeminiProjectRequired = errors.New("googleai google-gemini client: project is required") // ErrGoogleGeminiLocationRequired is returned when NewGoogleGeminiClient is called with empty location. ErrGoogleGeminiLocationRequired = errors.New("googleai google-gemini client: location is required") + // ErrNoCompletionInResponse is returned when a generate-content response contains no usable text. + ErrNoCompletionInResponse = errors.New("googleai: no completion in response") ) // Client calls the Gemini embeddings API via the Google Gen AI SDK. @@ -126,6 +132,60 @@ func (c *Client) CreateEmbeddingForQuery(ctx context.Context, input string) ([]f return c.embedWithTaskType(ctx, input, "RETRIEVAL_QUERY") } +// Translate generates content with the given system instruction and user text at +// temperature 0 (deterministic) using the configured model, returning the trimmed +// response text. It is the low-level call behind the translation enrichment; the +// service layer builds the prompt. +func (c *Client) Translate(ctx context.Context, systemPrompt, userText string) (string, error) { + userText = strings.TrimSpace(userText) + if userText == "" { + return "", ErrEmptyInput + } + + temperature := float32(0) + + resp, err := c.client.Models.GenerateContent(ctx, c.model, genai.Text(userText), &genai.GenerateContentConfig{ + Temperature: &temperature, + SystemInstruction: genai.NewContentFromText(systemPrompt, genai.RoleUser), + }) + if err != nil { + wrapped := fmt.Errorf("gemini generate content: %w", err) + + var apiErr genai.APIError + if errors.As(err, &apiErr) && (apiErr.Code == http.StatusTooManyRequests || apiErr.Status == "RESOURCE_EXHAUSTED") { + return "", huberrors.NewRateLimitError(genaiRetryAfter(apiErr), wrapped) + } + + return "", wrapped + } + + out := strings.TrimSpace(resp.Text()) + if out == "" { + return "", ErrNoCompletionInResponse + } + + return out, nil +} + +// genaiRetryAfter extracts the RetryInfo retryDelay from a RESOURCE_EXHAUSTED error's +// details, or 0 when absent or unparseable. +func genaiRetryAfter(apiErr genai.APIError) time.Duration { + for _, detail := range apiErr.Details { + typeName, _ := detail["@type"].(string) + if !strings.Contains(typeName, "RetryInfo") { + continue + } + + if delay, ok := detail["retryDelay"].(string); ok { + if dur, parseErr := time.ParseDuration(delay); parseErr == nil { + return dur + } + } + } + + return 0 +} + // embedWithTaskType calls the Gemini API with the given task type (e.g. RETRIEVAL_DOCUMENT, RETRIEVAL_QUERY). func (c *Client) embedWithTaskType(ctx context.Context, input, taskType string) ([]float32, error) { input = strings.TrimSpace(input) diff --git a/internal/googleai/client_test.go b/internal/googleai/client_test.go index a3c3e24f..8bab0e03 100644 --- a/internal/googleai/client_test.go +++ b/internal/googleai/client_test.go @@ -2,9 +2,16 @@ package googleai import ( "context" + "net/http" + "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/genai" + + "github.com/formbricks/hub/internal/huberrors" ) func TestNewGoogleGeminiClient_emptyProject_returnsError(t *testing.T) { @@ -46,3 +53,79 @@ func TestClient_CreateEmbedding_whitespaceInput_returnsErrEmptyInput(t *testing. _, err = client.CreateEmbedding(ctx, " \t\n ") assert.ErrorIs(t, err, ErrEmptyInput) } + +func TestGenaiRetryAfter(t *testing.T) { + const retryInfoType = "type.googleapis.com/google.rpc.RetryInfo" + + tests := []struct { + name string + details []map[string]any + want time.Duration + }{ + { + name: "retry info with parseable delay", + details: []map[string]any{{"@type": retryInfoType, "retryDelay": "17s"}}, + want: 17 * time.Second, + }, + { + name: "retry info among other details", + details: []map[string]any{ + {"@type": "type.googleapis.com/google.rpc.QuotaFailure"}, + {"@type": retryInfoType, "retryDelay": "1m30s"}, + }, + want: 90 * time.Second, + }, + { + name: "no retry info detail", + details: []map[string]any{{"@type": "type.googleapis.com/google.rpc.QuotaFailure"}}, + want: 0, + }, + { + name: "retry info with unparseable delay", + details: []map[string]any{{"@type": retryInfoType, "retryDelay": "soon"}}, + want: 0, + }, + { + name: "retry info missing delay", + details: []map[string]any{{"@type": retryInfoType}}, + want: 0, + }, + {name: "no details", details: nil, want: 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, genaiRetryAfter(genai.APIError{Details: tt.details})) + }) + } +} + +func TestTranslate_RateLimitReturnsRateLimitError(t *testing.T) { + ctx := context.Background() + + // Drive the real genai SDK against a stub endpoint returning a RESOURCE_EXHAUSTED error + // with a RetryInfo detail, so the SDK's own error decoding is exercised end to end. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"error":{"code":429,"status":"RESOURCE_EXHAUSTED","message":"quota",` + + `"details":[{"@type":"type.googleapis.com/google.rpc.RetryInfo","retryDelay":"23s"}]}}`)) + })) + t.Cleanup(server.Close) + + genaiClient, err := genai.NewClient(ctx, &genai.ClientConfig{ + APIKey: "test-key", + Backend: genai.BackendGeminiAPI, + HTTPOptions: genai.HTTPOptions{BaseURL: server.URL}, + }) + require.NoError(t, err) + + client := &Client{client: genaiClient, model: "gemini-2.5-flash"} + + _, err = client.Translate(ctx, "system prompt", "hello") + require.Error(t, err) + + var rateLimited *huberrors.RateLimitError + require.ErrorAs(t, err, &rateLimited) + assert.Equal(t, 23*time.Second, rateLimited.RetryAfter, "the RetryInfo retryDelay is honored") +} diff --git a/internal/huberrors/errors.go b/internal/huberrors/errors.go index d33acb49..e30b8c20 100644 --- a/internal/huberrors/errors.go +++ b/internal/huberrors/errors.go @@ -136,6 +136,36 @@ func (e *ConflictError) Is(target error) bool { return ok } +// ErrTranslationSuperseded is the sentinel for a feedback-record translation write skipped +// because the tenant's current target_language no longer matches the target the job was +// enqueued for — e.g. the target changed, or the job was enqueued from a stale tenant-settings +// cache read. The newer-target write owns the row; the stale write is a benign no-op, not a +// failure, so callers should record it as skipped rather than retry or fail. +var ErrTranslationSuperseded = &TranslationSupersededError{} + +// TranslationSupersededError is a sentinel for a stale-target translation write. It is +// deliberately distinct from NotFoundError (the record still exists) and ConflictError +// (nothing is wrong with the data) so an out-of-order translation is treated as a skip. +type TranslationSupersededError struct { + Message string +} + +// Error implements the error interface. +func (e *TranslationSupersededError) Error() string { + if e.Message != "" { + return e.Message + } + + return "translation superseded by a newer target language" +} + +// Is implements the error interface for error comparison. +func (e *TranslationSupersededError) Is(target error) bool { + _, ok := target.(*TranslationSupersededError) + + return ok +} + // ErrTenantWriteConflict is the sentinel for tenant write coordination conflicts: // a tenant-owned write rejected because a tenant data purge is in progress, or a // purge that could not acquire its lock while tenant-owned writes were in flight. diff --git a/internal/huberrors/rate_limit.go b/internal/huberrors/rate_limit.go new file mode 100644 index 00000000..ebc6b18f --- /dev/null +++ b/internal/huberrors/rate_limit.go @@ -0,0 +1,32 @@ +package huberrors + +import ( + "fmt" + "time" +) + +// RateLimitError marks a provider rate-limit response (HTTP 429 / RESOURCE_EXHAUSTED). +// RetryAfter is the provider-suggested delay before retrying, or 0 when unknown. Enrichment +// workers snooze for RetryAfter instead of consuming a retry attempt, so a burst against a +// rate-limited model defers work rather than dropping it. +type RateLimitError struct { + RetryAfter time.Duration + Err error +} + +// NewRateLimitError wraps err as a rate-limit error carrying the provider's retry-after hint +// (0 when the provider gave none). +func NewRateLimitError(retryAfter time.Duration, err error) *RateLimitError { + return &RateLimitError{RetryAfter: retryAfter, Err: err} +} + +func (e *RateLimitError) Error() string { + if e.Err != nil { + return fmt.Sprintf("rate limited (retry after %s): %v", e.RetryAfter, e.Err) + } + + return fmt.Sprintf("rate limited (retry after %s)", e.RetryAfter) +} + +// Unwrap exposes the underlying provider error for errors.Is/As. +func (e *RateLimitError) Unwrap() error { return e.Err } diff --git a/internal/huberrors/rate_limit_test.go b/internal/huberrors/rate_limit_test.go new file mode 100644 index 00000000..67a90c99 --- /dev/null +++ b/internal/huberrors/rate_limit_test.go @@ -0,0 +1,31 @@ +package huberrors + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRateLimitError_WrapsAndUnwraps(t *testing.T) { + cause := errors.New("429 from provider") + err := NewRateLimitError(12*time.Second, cause) + + require.Equal(t, 12*time.Second, err.RetryAfter) + require.ErrorIs(t, err, cause, "Unwrap exposes the underlying provider error") + assert.Contains(t, err.Error(), "12s") + assert.Contains(t, err.Error(), "429 from provider") + + var rateLimited *RateLimitError + assert.ErrorAs(t, err, &rateLimited) +} + +func TestRateLimitError_NilCause(t *testing.T) { + err := NewRateLimitError(0, nil) + + require.NoError(t, err.Unwrap(), "no cause unwraps to nil") + assert.Contains(t, err.Error(), "rate limited") + assert.NotContains(t, err.Error(), "%!", "the message must not contain a bad fmt verb") +} diff --git a/internal/models/feedback_records.go b/internal/models/feedback_records.go index b44eefdc..a3b02ad7 100644 --- a/internal/models/feedback_records.go +++ b/internal/models/feedback_records.go @@ -153,6 +153,10 @@ type FeedbackRecord struct { UserID *string `json:"user_id,omitempty"` TenantID string `json:"tenant_id"` SubmissionID string `json:"submission_id"` // mandatory; never null + // Language-enrichment outputs (ENG-1255): server-generated, read-only. NULL until + // the record is translated into the tenant's configured target language. + ValueTextTranslated *string `json:"value_text_translated,omitempty"` + TranslationLangKey *string `json:"translation_lang_key,omitempty"` } // CreateFeedbackRecordRequest represents the request to create a feedback record. @@ -177,6 +181,13 @@ type CreateFeedbackRecordRequest struct { SubmissionID string `json:"submission_id" validate:"required,no_null_bytes,min=1,max=255"` } +// TranslationBackfillTarget is a feedback record that needs (re)translation to its +// tenant's currently-configured target language, returned by the backfill query. +type TranslationBackfillTarget struct { + FeedbackRecordID uuid.UUID + TargetLang string +} + // UpdateFeedbackRecordRequest represents the request to update a feedback record // Only value fields, metadata, language, and user_id can be updated. type UpdateFeedbackRecordRequest struct { diff --git a/internal/observability/aggregate.go b/internal/observability/aggregate.go index 74ed0de2..33c4fd61 100644 --- a/internal/observability/aggregate.go +++ b/internal/observability/aggregate.go @@ -6,18 +6,19 @@ import ( "go.opentelemetry.io/otel/metric" ) -// Metrics holds all hub metric collectors (events, webhooks, embeddings, cache). +// Metrics holds all hub metric collectors (events, webhooks, embeddings, translation, cache). // When metrics are disabled, Metrics is nil and all fields are nil. -// Components that accept an interface (EventMetrics, WebhookMetrics, EmbeddingMetrics, CacheMetrics) -// can receive the corresponding field; they already handle nil. +// Components that accept an interface (EventMetrics, WebhookMetrics, EmbeddingMetrics, +// TranslationMetrics, CacheMetrics) can receive the corresponding field; they already handle nil. type Metrics struct { - Events EventMetrics - Webhooks WebhookMetrics - Embeddings EmbeddingMetrics - Cache CacheMetrics + Events EventMetrics + Webhooks WebhookMetrics + Embeddings EmbeddingMetrics + Translation TranslationMetrics + Cache CacheMetrics } -// NewMetrics creates EventMetrics, WebhookMetrics, and EmbeddingMetrics from the given meter. +// NewMetrics creates EventMetrics, WebhookMetrics, EmbeddingMetrics, TranslationMetrics, and CacheMetrics from the given meter. // Returns (nil, nil) when meter is nil (metrics disabled). func NewMetrics(meter metric.Meter) (*Metrics, error) { if meter == nil { @@ -40,15 +41,21 @@ func NewMetrics(meter metric.Meter) (*Metrics, error) { return nil, fmt.Errorf("embedding metrics: %w", err) } + translation, err := NewTranslationMetrics(meter) + if err != nil { + return nil, fmt.Errorf("translation metrics: %w", err) + } + cache, err := NewCacheMetrics(meter) if err != nil { return nil, fmt.Errorf("cache metrics: %w", err) } return &Metrics{ - Events: events, - Webhooks: webhooks, - Embeddings: embeddings, - Cache: cache, + Events: events, + Webhooks: webhooks, + Embeddings: embeddings, + Translation: translation, + Cache: cache, }, nil } diff --git a/internal/observability/cache.go b/internal/observability/cache.go index 4323a84a..880bda30 100644 --- a/internal/observability/cache.go +++ b/internal/observability/cache.go @@ -28,7 +28,7 @@ func NewCacheMetrics(meter metric.Meter) (CacheMetrics, error) { } hitDesc := "Number of cache lookups that returned a cached value. " + - "Label cache: search_query_embedding. " + + "Labels cache: search_query_embedding, tenant_settings. " + "Hit ratio = rate(hits) / (rate(hits) + rate(misses)) per cache." hits, err := meter.Int64Counter( @@ -39,7 +39,7 @@ func NewCacheMetrics(meter metric.Meter) (CacheMetrics, error) { } missDesc := "Number of cache lookups that missed and triggered a load from the backing store. " + - "Label cache: search_query_embedding." + "Labels cache: search_query_embedding, tenant_settings." misses, err := meter.Int64Counter( MetricNameCacheMisses, metric.WithDescription(missDesc), metric.WithUnit("1"), diff --git a/internal/observability/names.go b/internal/observability/names.go index cb16de89..82711718 100644 --- a/internal/observability/names.go +++ b/internal/observability/names.go @@ -25,6 +25,13 @@ const ( MetricNameEmbeddingWorkerErrors = "hub_embedding_worker_errors_total" MetricNameEmbeddingDuration = "hub_embedding_duration_seconds" + // MetricNameTranslationJobsEnqueued and related translation pipeline metrics. + MetricNameTranslationJobsEnqueued = "hub_translation_jobs_enqueued_total" + MetricNameTranslationProviderErrors = "hub_translation_provider_errors_total" + MetricNameTranslationOutcomes = "hub_translation_outcomes_total" + MetricNameTranslationWorkerErrors = "hub_translation_worker_errors_total" + MetricNameTranslationDuration = "hub_translation_duration_seconds" + MetricNameCacheHits = "hub_cache_hits_total" MetricNameCacheMisses = "hub_cache_misses_total" ) @@ -102,6 +109,44 @@ func AllowedEmbeddingOutcomeStatus(status string) bool { // AllowedEmbeddingWorkerReason returns true if reason is allowed for embedding worker errors. func AllowedEmbeddingWorkerReason(reason string) bool { return allowedEmbeddingWorkerReasons[reason] } +// allowedTranslationProviderReasons for hub_translation_provider_errors_total. +var allowedTranslationProviderReasons = map[string]bool{ + "settings_read_failed": true, + "enqueue_failed": true, +} + +// allowedTranslationOutcomeStatuses for hub_translation_outcomes_total and hub_translation_duration_seconds. +var allowedTranslationOutcomeStatuses = map[string]bool{ + "success": true, + "retry": true, + "failed_final": true, + "skipped": true, +} + +// allowedTranslationWorkerReasons for hub_translation_worker_errors_total. +var allowedTranslationWorkerReasons = map[string]bool{ + "translation_api_failed": true, + "get_record_failed": true, + "update_failed": true, + "tenant_write_conflict": true, + "rate_limited": true, +} + +// AllowedTranslationProviderReason returns true if reason is allowed for translation provider errors. +func AllowedTranslationProviderReason(reason string) bool { + return allowedTranslationProviderReasons[reason] +} + +// AllowedTranslationOutcomeStatus returns true if status is allowed for translation outcomes. +func AllowedTranslationOutcomeStatus(status string) bool { + return allowedTranslationOutcomeStatuses[status] +} + +// AllowedTranslationWorkerReason returns true if reason is allowed for translation worker errors. +func AllowedTranslationWorkerReason(reason string) bool { + return allowedTranslationWorkerReasons[reason] +} + // AllowedProviderReason returns true if reason is an allowed provider error reason. func AllowedProviderReason(reason string) bool { return allowedProviderReasons[reason] } @@ -117,6 +162,7 @@ func AllowedDispatchReason(reason string) bool { return allowedDispatchReasons[r // allowedCacheNames for hub_cache_hits_total / hub_cache_misses_total (bounded cardinality). var allowedCacheNames = map[string]bool{ "search_query_embedding": true, + "tenant_settings": true, } // AllowedCacheName returns true if name is an allowed cache name for metrics. diff --git a/internal/observability/translation.go b/internal/observability/translation.go new file mode 100644 index 00000000..dea447b4 --- /dev/null +++ b/internal/observability/translation.go @@ -0,0 +1,118 @@ +package observability + +import ( + "context" + "fmt" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" +) + +// TranslationMetrics records translation pipeline metrics (provider, worker). +// Methods accept ctx for future exemplar support. Mirrors EmbeddingMetrics. +type TranslationMetrics interface { + RecordJobsEnqueued(ctx context.Context, count int64) + RecordProviderError(ctx context.Context, reason string) + RecordTranslationOutcome(ctx context.Context, status string) + RecordWorkerError(ctx context.Context, reason string) + RecordTranslationDuration(ctx context.Context, duration time.Duration, status string) +} + +// translationMetrics implements TranslationMetrics. +type translationMetrics struct { + jobsEnqueued metric.Int64Counter + providerErrors metric.Int64Counter + outcomes metric.Int64Counter + workerErrors metric.Int64Counter + duration metric.Float64Histogram +} + +// NewTranslationMetrics creates TranslationMetrics. Returns (nil, nil) when meter is nil (metrics disabled). +func NewTranslationMetrics(meter metric.Meter) (TranslationMetrics, error) { + if meter == nil { + //nolint:nilnil // intentional: callers use "if metrics != nil" when metrics disabled + return nil, nil + } + + jobsEnqueued, err := meter.Int64Counter( + MetricNameTranslationJobsEnqueued, + metric.WithDescription("Total translation jobs enqueued"), + ) + if err != nil { + return nil, fmt.Errorf("create translation jobs enqueued counter: %w", err) + } + + providerErrors, err := meter.Int64Counter( + MetricNameTranslationProviderErrors, + metric.WithDescription("Total translation provider errors (settings/enqueue failures)"), + ) + if err != nil { + return nil, fmt.Errorf("create translation provider errors counter: %w", err) + } + + outcomes, err := meter.Int64Counter( + MetricNameTranslationOutcomes, + metric.WithDescription("Total translation job outcomes by status"), + ) + if err != nil { + return nil, fmt.Errorf("create translation outcomes counter: %w", err) + } + + workerErrors, err := meter.Int64Counter( + MetricNameTranslationWorkerErrors, + metric.WithDescription("Total translation worker errors (get record, provider, update)"), + ) + if err != nil { + return nil, fmt.Errorf("create translation worker errors counter: %w", err) + } + + duration, err := meter.Float64Histogram( + MetricNameTranslationDuration, + metric.WithDescription("Translation job duration (seconds)"), + metric.WithUnit("s"), + ) + if err != nil { + return nil, fmt.Errorf("create translation duration histogram: %w", err) + } + + return &translationMetrics{ + jobsEnqueued: jobsEnqueued, + providerErrors: providerErrors, + outcomes: outcomes, + workerErrors: workerErrors, + duration: duration, + }, nil +} + +func (t *translationMetrics) RecordJobsEnqueued(ctx context.Context, count int64) { + t.jobsEnqueued.Add(ctx, count) +} + +func (t *translationMetrics) RecordProviderError(ctx context.Context, reason string) { + reason = NormalizeReason(reason, AllowedTranslationProviderReason) + t.providerErrors.Add(ctx, 1, metric.WithAttributes(attribute.String(AttrReason, reason))) +} + +func (t *translationMetrics) RecordTranslationOutcome(ctx context.Context, status string) { + status = normalizeTranslationStatus(status) + t.outcomes.Add(ctx, 1, metric.WithAttributes(attribute.String(AttrStatus, status))) +} + +func (t *translationMetrics) RecordWorkerError(ctx context.Context, reason string) { + reason = NormalizeReason(reason, AllowedTranslationWorkerReason) + t.workerErrors.Add(ctx, 1, metric.WithAttributes(attribute.String(AttrReason, reason))) +} + +func (t *translationMetrics) RecordTranslationDuration(ctx context.Context, duration time.Duration, status string) { + status = normalizeTranslationStatus(status) + t.duration.Record(ctx, duration.Seconds(), metric.WithAttributes(attribute.String(AttrStatus, status))) +} + +func normalizeTranslationStatus(status string) string { + if AllowedTranslationOutcomeStatus(status) { + return status + } + + return "other" +} diff --git a/internal/observability/translation_test.go b/internal/observability/translation_test.go new file mode 100644 index 00000000..d0c1928d --- /dev/null +++ b/internal/observability/translation_test.go @@ -0,0 +1,139 @@ +package observability + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +func TestTranslationAllowedReasonsAndStatuses(t *testing.T) { + for _, reason := range []string{"settings_read_failed", "enqueue_failed"} { + assert.True(t, AllowedTranslationProviderReason(reason), reason) + assert.Equal(t, reason, NormalizeReason(reason, AllowedTranslationProviderReason)) + } + + workerReasons := []string{ + "translation_api_failed", "get_record_failed", "update_failed", "tenant_write_conflict", "rate_limited", + } + for _, reason := range workerReasons { + assert.True(t, AllowedTranslationWorkerReason(reason), reason) + assert.Equal(t, reason, NormalizeReason(reason, AllowedTranslationWorkerReason)) + } + + for _, status := range []string{"success", "retry", "failed_final", "skipped"} { + assert.True(t, AllowedTranslationOutcomeStatus(status), status) + assert.Equal(t, status, normalizeTranslationStatus(status)) + } + + // Unknown values normalize to a bounded "other" bucket (keeps metric cardinality fixed). + assert.False(t, AllowedTranslationProviderReason("nope")) + assert.Equal(t, "other", NormalizeReason("nope", AllowedTranslationProviderReason)) + assert.Equal(t, "other", normalizeTranslationStatus("nope")) +} + +func TestNewTranslationMetricsNilMeterDisabled(t *testing.T) { + metrics, err := NewTranslationMetrics(nil) + require.NoError(t, err) + assert.Nil(t, metrics, "a nil meter disables metrics") +} + +func TestNewMetricsIncludesTranslation(t *testing.T) { + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + + metrics, err := NewMetrics(provider.Meter("test")) + require.NoError(t, err) + require.NotNil(t, metrics) + assert.NotNil(t, metrics.Translation, "aggregate Metrics wires up the translation collector") + + disabled, err := NewMetrics(nil) + require.NoError(t, err) + assert.Nil(t, disabled, "a nil meter disables the aggregate") +} + +func TestTranslationMetricsRecordEmitsDataPoints(t *testing.T) { + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + + metrics, err := NewTranslationMetrics(provider.Meter("test")) + require.NoError(t, err) + require.NotNil(t, metrics) + + ctx := context.Background() + metrics.RecordJobsEnqueued(ctx, 2) + metrics.RecordProviderError(ctx, "enqueue_failed") + metrics.RecordProviderError(ctx, "bogus") // unknown reason -> normalized to "other" + metrics.RecordTranslationOutcome(ctx, "success") + metrics.RecordWorkerError(ctx, "tenant_write_conflict") + metrics.RecordTranslationDuration(ctx, 150*time.Millisecond, "success") + + var collected metricdata.ResourceMetrics + require.NoError(t, reader.Collect(ctx, &collected)) + + assert.Equal(t, int64(2), counterValue(collected, MetricNameTranslationJobsEnqueued, "", "")) + assert.Equal(t, int64(1), counterValue(collected, MetricNameTranslationProviderErrors, AttrReason, "enqueue_failed")) + assert.Equal(t, int64(1), counterValue(collected, MetricNameTranslationProviderErrors, AttrReason, "other")) + assert.Equal(t, int64(1), counterValue(collected, MetricNameTranslationOutcomes, AttrStatus, "success")) + assert.Equal(t, int64(1), counterValue(collected, MetricNameTranslationWorkerErrors, AttrReason, "tenant_write_conflict")) + assert.Equal(t, uint64(1), histogramCount(collected, MetricNameTranslationDuration, "success")) +} + +// counterValue returns the value of an Int64 counter datapoint matching name and the +// optional attribute key/value. An empty attrKey matches a datapoint with no attributes. +func counterValue(data metricdata.ResourceMetrics, name, attrKey, attrVal string) int64 { + for _, scope := range data.ScopeMetrics { + for _, metricItem := range scope.Metrics { + if metricItem.Name != name { + continue + } + + sum, ok := metricItem.Data.(metricdata.Sum[int64]) + if !ok { + continue + } + + for _, point := range sum.DataPoints { + if attrKey == "" { + return point.Value + } + + if value, present := point.Attributes.Value(attribute.Key(attrKey)); present && value.AsString() == attrVal { + return point.Value + } + } + } + } + + return 0 +} + +// histogramCount returns the observation count of a float64 histogram datapoint matching +// name and the given status attribute. +func histogramCount(data metricdata.ResourceMetrics, name, status string) uint64 { + for _, scope := range data.ScopeMetrics { + for _, metricItem := range scope.Metrics { + if metricItem.Name != name { + continue + } + + hist, ok := metricItem.Data.(metricdata.Histogram[float64]) + if !ok { + continue + } + + for _, point := range hist.DataPoints { + if value, present := point.Attributes.Value(attribute.Key(AttrStatus)); present && value.AsString() == status { + return point.Count + } + } + } + } + + return 0 +} diff --git a/internal/openai/client.go b/internal/openai/client.go index dd2eeb20..cf2ebd48 100644 --- a/internal/openai/client.go +++ b/internal/openai/client.go @@ -1,16 +1,21 @@ -// Package openai provides a thin wrapper around the official OpenAI Go SDK for embeddings. +// Package openai provides a thin wrapper around the official OpenAI Go SDK for +// embeddings and chat completions (used for translation). package openai import ( "context" "errors" "fmt" + "net/http" + "strconv" "strings" + "time" openaisdk "github.com/openai/openai-go/v3" "github.com/openai/openai-go/v3/option" "github.com/openai/openai-go/v3/packages/param" + "github.com/formbricks/hub/internal/huberrors" "github.com/formbricks/hub/internal/models" "github.com/formbricks/hub/pkg/embeddings" ) @@ -24,6 +29,8 @@ var ( ErrNoEmbeddingInResponse = errors.New("openai: no embedding in response") // ErrDimensionMismatch is returned when the response embedding length does not match configured dimensions. ErrDimensionMismatch = errors.New("openai: embedding dimension mismatch") + // ErrNoCompletionInResponse is returned when a chat completion response contains no usable text. + ErrNoCompletionInResponse = errors.New("openai: no completion in response") ) // Client calls the OpenAI embeddings API via the official SDK. @@ -143,3 +150,59 @@ func (c *Client) CreateEmbedding(ctx context.Context, input string) ([]float32, func (c *Client) CreateEmbeddingForQuery(ctx context.Context, input string) ([]float32, error) { return c.CreateEmbedding(ctx, input) } + +// Translate sends a chat completion with the given system prompt and user text at +// temperature 0 (deterministic) using the configured model, returning the trimmed +// assistant text. It is the low-level call behind the translation enrichment; the +// service layer builds the prompt. +func (c *Client) Translate(ctx context.Context, systemPrompt, userText string) (string, error) { + userText = strings.TrimSpace(userText) + if userText == "" { + return "", ErrEmptyInput + } + + resp, err := c.sdk.Chat.Completions.New(ctx, openaisdk.ChatCompletionNewParams{ + Model: c.model, + Temperature: param.NewOpt(0.0), + Messages: []openaisdk.ChatCompletionMessageParamUnion{ + openaisdk.SystemMessage(systemPrompt), + openaisdk.UserMessage(userText), + }, + }) + if err != nil { + wrapped := fmt.Errorf("openai chat completion: %w", err) + + var apiErr *openaisdk.Error + if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusTooManyRequests { + return "", huberrors.NewRateLimitError(openaiRetryAfter(apiErr), wrapped) + } + + return "", wrapped + } + + if len(resp.Choices) == 0 { + return "", ErrNoCompletionInResponse + } + + out := strings.TrimSpace(resp.Choices[0].Message.Content) + if out == "" { + return "", ErrNoCompletionInResponse + } + + return out, nil +} + +// openaiRetryAfter reads the Retry-After header (delta-seconds) from a 429 response, or 0 +// when absent or not an integer-seconds value. +func openaiRetryAfter(apiErr *openaisdk.Error) time.Duration { + if apiErr == nil || apiErr.Response == nil { + return 0 + } + + secs, err := strconv.Atoi(apiErr.Response.Header.Get("Retry-After")) + if err != nil || secs < 0 { + return 0 + } + + return time.Duration(secs) * time.Second +} diff --git a/internal/openai/client_test.go b/internal/openai/client_test.go index ccfc6495..7c1f59a7 100644 --- a/internal/openai/client_test.go +++ b/internal/openai/client_test.go @@ -7,9 +7,12 @@ import ( "net/http/httptest" "sync/atomic" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/formbricks/hub/internal/huberrors" ) type embeddingRequest struct { @@ -100,3 +103,89 @@ func TestCreateEmbedding_UsesEnvironmentBaseURLWhenExplicitBaseURLIsUnset(t *tes assert.Equal(t, []float32{3, 4}, embedding) assert.Equal(t, int32(1), envHits.Load()) } + +// newChatCompletionServer drives the real SDK against a stub /v1/chat/completions endpoint so +// the translation error paths exercise the SDK's own response decoding, not a hand-built error. +func newChatCompletionServer(t *testing.T, handler http.HandlerFunc) *httptest.Server { + t.Helper() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v1/chat/completions", r.URL.Path) + handler(w, r) + })) + t.Cleanup(server.Close) + + return server +} + +func TestTranslate_RateLimitReturnsRateLimitError(t *testing.T) { + // x-should-retry:false stops the SDK's own retry/backoff, so the 429 surfaces immediately. + server := newChatCompletionServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Retry-After", "12") + w.Header().Set("X-Should-Retry", "false") + w.WriteHeader(http.StatusTooManyRequests) + }) + + client := NewClient("sk-test", WithBaseURL(server.URL+"/v1"), WithModel("test-model")) + + _, err := client.Translate(context.Background(), "system prompt", "hello") + require.Error(t, err) + + var rateLimited *huberrors.RateLimitError + require.ErrorAs(t, err, &rateLimited) + assert.Equal(t, 12*time.Second, rateLimited.RetryAfter, "the Retry-After header is honored") +} + +func TestTranslate_RateLimitWithoutRetryAfterHeader(t *testing.T) { + server := newChatCompletionServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Should-Retry", "false") + w.WriteHeader(http.StatusTooManyRequests) + }) + + client := NewClient("sk-test", WithBaseURL(server.URL+"/v1"), WithModel("test-model")) + + _, err := client.Translate(context.Background(), "system prompt", "hello") + + var rateLimited *huberrors.RateLimitError + require.ErrorAs(t, err, &rateLimited) + assert.Equal(t, time.Duration(0), rateLimited.RetryAfter, "no header means no hint, not a parse error") +} + +func TestTranslate_NonRateLimitErrorIsNotWrapped(t *testing.T) { + server := newChatCompletionServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + }) + + client := NewClient("sk-test", WithBaseURL(server.URL+"/v1"), WithModel("test-model")) + + _, err := client.Translate(context.Background(), "system prompt", "hello") + require.Error(t, err) + + var rateLimited *huberrors.RateLimitError + assert.NotErrorAs(t, err, &rateLimited, "a non-429 error must not be classified as rate-limited") +} + +func TestTranslate_Success(t *testing.T) { + server := newChatCompletionServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + + if err := json.NewEncoder(w).Encode(map[string]any{ + "id": "chatcmpl-test", + "object": "chat.completion", + "model": "test-model", + "choices": []map[string]any{{ + "index": 0, + "finish_reason": "stop", + "message": map[string]any{"role": "assistant", "content": " Hola mundo "}, + }}, + }); err != nil { + t.Errorf("encode response body: %v", err) + } + }) + + client := NewClient("sk-test", WithBaseURL(server.URL+"/v1"), WithModel("test-model")) + + out, err := client.Translate(context.Background(), "system prompt", "hello") + require.NoError(t, err) + assert.Equal(t, "Hola mundo", out, "the assistant message is returned trimmed") +} diff --git a/internal/repository/feedback_records_repository.go b/internal/repository/feedback_records_repository.go index aabf565b..1a5b61b9 100644 --- a/internal/repository/feedback_records_repository.go +++ b/internal/repository/feedback_records_repository.go @@ -31,6 +31,18 @@ func NewFeedbackRecordsRepository(db *pgxpool.Pool) *FeedbackRecordsRepository { return &FeedbackRecordsRepository{db: db} } +// feedbackRecordColumns is the canonical SELECT/RETURNING column list for a +// FeedbackRecord, in the exact order scanFeedbackRecord reads it. Together they are +// the single source of truth for materializing a FeedbackRecord, so column order +// and scan order cannot drift across the Create/Get/List/Update read paths (a +// silent runtime scan error otherwise). It excludes derived rows like embeddings. +const feedbackRecordColumns = `id, collected_at, created_at, updated_at, + source_type, source_id, source_name, + field_id, field_label, field_type, field_group_id, field_group_label, + value_text, value_number, value_boolean, value_date, + metadata, language, user_id, tenant_id, submission_id, + value_text_translated, translation_lang_key` + // Create inserts a new feedback record. func (r *FeedbackRecordsRepository) Create(ctx context.Context, req *models.CreateFeedbackRecordRequest) (*models.FeedbackRecord, error) { collectedAt := time.Now() @@ -53,28 +65,15 @@ func (r *FeedbackRecordsRepository) Create(ctx context.Context, req *models.Crea ) SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18 WHERE ` + tenantWriteLockGate(lockKeyParam) + ` - RETURNING id, collected_at, created_at, updated_at, - source_type, source_id, source_name, - field_id, field_label, field_type, field_group_id, field_group_label, - value_text, value_number, value_boolean, value_date, - metadata, language, user_id, tenant_id, submission_id - ` - - var record models.FeedbackRecord + RETURNING ` + feedbackRecordColumns - err := r.db.QueryRow(ctx, query, + record, err := scanFeedbackRecord(r.db.QueryRow(ctx, query, collectedAt, req.SourceType, req.SourceID, req.SourceName, req.FieldID, req.FieldLabel, req.FieldType, req.FieldGroupID, req.FieldGroupLabel, req.ValueText, req.ValueNumber, req.ValueBoolean, req.ValueDate, req.Metadata, req.Language, req.UserID, req.TenantID, req.SubmissionID, TenantWriteLockKey(req.TenantID), - ).Scan( - &record.ID, &record.CollectedAt, &record.CreatedAt, &record.UpdatedAt, - &record.SourceType, &record.SourceID, &record.SourceName, - &record.FieldID, &record.FieldLabel, &record.FieldType, &record.FieldGroupID, &record.FieldGroupLabel, - &record.ValueText, &record.ValueNumber, &record.ValueBoolean, &record.ValueDate, - &record.Metadata, &record.Language, &record.UserID, &record.TenantID, &record.SubmissionID, - ) + )) if err != nil { var pgErr *pgconn.PgError if errors.As(err, &pgErr) && pgErr.Code == uniqueViolationSQLState { @@ -88,7 +87,7 @@ func (r *FeedbackRecordsRepository) Create(ctx context.Context, req *models.Crea return nil, fmt.Errorf("failed to create feedback record: %w", err) } - return &record, nil + return record, nil } // resolveFeedbackRecordTenant reads the tenant boundary of a feedback record @@ -131,25 +130,11 @@ func lockFeedbackRecordTenantShared(ctx context.Context, dbTx tenantWriteTx, id // GetByID retrieves a single feedback record by ID. Embedding is not selected (API/worker reads stay lean). func (r *FeedbackRecordsRepository) GetByID(ctx context.Context, id uuid.UUID) (*models.FeedbackRecord, error) { - query := ` - SELECT id, collected_at, created_at, updated_at, - source_type, source_id, source_name, - field_id, field_label, field_type, field_group_id, field_group_label, - value_text, value_number, value_boolean, value_date, - metadata, language, user_id, tenant_id, submission_id + query := `SELECT ` + feedbackRecordColumns + ` FROM feedback_records - WHERE id = $1 - ` - - var record models.FeedbackRecord + WHERE id = $1` - err := r.db.QueryRow(ctx, query, id).Scan( - &record.ID, &record.CollectedAt, &record.CreatedAt, &record.UpdatedAt, - &record.SourceType, &record.SourceID, &record.SourceName, - &record.FieldID, &record.FieldLabel, &record.FieldType, &record.FieldGroupID, &record.FieldGroupLabel, - &record.ValueText, &record.ValueNumber, &record.ValueBoolean, &record.ValueDate, - &record.Metadata, &record.Language, &record.UserID, &record.TenantID, &record.SubmissionID, - ) + record, err := scanFeedbackRecord(r.db.QueryRow(ctx, query, id)) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return nil, huberrors.NewNotFoundError("feedback record", "feedback record not found") @@ -158,7 +143,150 @@ func (r *FeedbackRecordsRepository) GetByID(ctx context.Context, id uuid.UUID) ( return nil, fmt.Errorf("failed to get feedback record: %w", err) } - return &record, nil + return record, nil +} + +// SetTranslation stores the translated text and the target locale it was produced in +// for a feedback record. The write is scoped to the record's tenant via the shared +// tenant write lock (so it cannot race a tenant data purge) and does NOT publish a +// domain event: translation is a derived enrichment, not a record edit, and +// re-publishing would loop the enrichment pipeline. A missing record (deleted or +// purged between read and write) returns NotFound. translated may be nil. +// +// Setting a translation (translated != nil) is conditional: it lands only while the tenant's +// current target_language still equals langKey, otherwise it returns +// huberrors.ErrTranslationSuperseded. This makes the write atomic w.r.t. a concurrent target +// change and immune to a stale settings-cache read, so an out-of-order stale-target job cannot +// clobber a newer translation. Clearing (translated == nil) is unconditional. +func (r *FeedbackRecordsRepository) SetTranslation( + ctx context.Context, feedbackRecordID uuid.UUID, translated *string, langKey string, +) error { + return withTenantWritePoolTx(ctx, r.db, nil, func(dbTx tenantWriteTx) error { + // Translation rides the feedback record's tenant boundary; resolve and lock + // it so the write cannot race a tenant data purge. + if _, err := lockFeedbackRecordTenantShared(ctx, dbTx, feedbackRecordID); err != nil { + return err + } + + // Clearing (translated == nil) nulls both columns unconditionally: an emptied + // value_text must drop any stale translation regardless of the tenant's target, + // and a lang key has no meaning without a translation. + if translated == nil { + if _, err := dbTx.Exec(ctx, ` + UPDATE feedback_records + SET value_text_translated = NULL, translation_lang_key = NULL, updated_at = NOW() + WHERE id = $1`, + feedbackRecordID, + ); err != nil { + return fmt.Errorf("clear feedback record translation: %w", err) + } + + return nil + } + + // Setting a translation persists only while the tenant's CURRENT target_language still + // equals the locale this translation was produced for. Joining tenant_settings in the + // write keeps it atomic against a concurrent target change and a stale settings-cache + // read at enqueue: a stale-target job (an older target finishing after a newer-target + // job or backfill) matches no row and no-ops instead of clobbering the current value. + tag, err := dbTx.Exec(ctx, ` + UPDATE feedback_records fr + SET value_text_translated = $2, translation_lang_key = $3, updated_at = NOW() + FROM tenant_settings ts + WHERE fr.id = $1 + AND ts.tenant_id = fr.tenant_id + AND ts.settings->>'target_language' = $3`, + feedbackRecordID, *translated, langKey, + ) + if err != nil { + return fmt.Errorf("set feedback record translation: %w", err) + } + + // The record exists (locked above), so zero rows means the tenant's current target no + // longer equals langKey: a newer-target write owns the row. Signal a benign skip. + if tag.RowsAffected() == 0 { + return huberrors.ErrTranslationSuperseded + } + + return nil + }) +} + +// translationBackfillSelectSQL selects feedback records that need (re)translation: text +// fields with non-empty value_text whose tenant has a target language configured and whose +// stored translation_lang_key differs from that target (never translated, or translated to +// a now-stale target). It joins tenant_settings for the per-tenant target. Callers append +// ordering / keyset / limit clauses. +const translationBackfillSelectSQL = ` + SELECT fr.id, ts.settings->>'target_language' + FROM feedback_records fr + JOIN tenant_settings ts ON ts.tenant_id = fr.tenant_id + WHERE fr.field_type = 'text' + AND fr.value_text IS NOT NULL AND btrim(fr.value_text) <> '' + AND COALESCE(ts.settings->>'target_language', '') <> '' + AND fr.translation_lang_key IS DISTINCT FROM ts.settings->>'target_language'` + +// ListTranslationBackfillTargets returns one keyset page (fr.id > afterID, ordered by id, at +// most limit rows) of feedback records across all tenants that need (re)translation. Used by +// the one-off global backfill command. Pass uuid.Nil as afterID for the first page. +func (r *FeedbackRecordsRepository) ListTranslationBackfillTargets( + ctx context.Context, afterID uuid.UUID, limit int, +) ([]models.TranslationBackfillTarget, error) { + const query = translationBackfillSelectSQL + ` + AND fr.id > $1 + ORDER BY fr.id + LIMIT $2` + + rows, err := r.db.Query(ctx, query, afterID, limit) + if err != nil { + return nil, fmt.Errorf("query translation backfill targets: %w", err) + } + + return scanTranslationBackfillTargets(rows) +} + +// ListTranslationBackfillTargetsForTenant returns one keyset page (fr.id > afterID, ordered +// by id, at most limit rows) of a single tenant's records that need (re)translation. The +// tenant filter + pagination let the settings-triggered backfill worker stream a large +// tenant without materializing every target at once. Pass uuid.Nil as afterID for the +// first page (every UUIDv7 id sorts after it). +func (r *FeedbackRecordsRepository) ListTranslationBackfillTargetsForTenant( + ctx context.Context, tenantID string, afterID uuid.UUID, limit int, +) ([]models.TranslationBackfillTarget, error) { + const query = translationBackfillSelectSQL + ` + AND fr.tenant_id = $1 + AND fr.id > $2 + ORDER BY fr.id + LIMIT $3` + + rows, err := r.db.Query(ctx, query, tenantID, afterID, limit) + if err != nil { + return nil, fmt.Errorf("query translation backfill targets for tenant: %w", err) + } + + return scanTranslationBackfillTargets(rows) +} + +// scanTranslationBackfillTargets collects (id, target_language) rows and closes rows. +func scanTranslationBackfillTargets(rows pgx.Rows) ([]models.TranslationBackfillTarget, error) { + defer rows.Close() + + var targets []models.TranslationBackfillTarget + + for rows.Next() { + var target models.TranslationBackfillTarget + if err := rows.Scan(&target.FeedbackRecordID, &target.TargetLang); err != nil { + return nil, fmt.Errorf("scan translation backfill target: %w", err) + } + + targets = append(targets, target) + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate translation backfill targets: %w", err) + } + + return targets, nil } // buildFilterConditions builds WHERE clause conditions and arguments from filters. @@ -234,12 +362,7 @@ func buildFilterConditions(filters *models.ListFeedbackRecordsFilters) (whereCla return whereClause, args } -const feedbackRecordsListSelect = ` - SELECT id, collected_at, created_at, updated_at, - source_type, source_id, source_name, - field_id, field_label, field_type, field_group_id, field_group_label, - value_text, value_number, value_boolean, value_date, - metadata, language, user_id, tenant_id, submission_id +const feedbackRecordsListSelect = `SELECT ` + feedbackRecordColumns + ` FROM feedback_records ` @@ -394,12 +517,8 @@ func buildUpdateQuery( UPDATE feedback_records SET %s WHERE id = $%d AND tenant_id = $%d - RETURNING id, collected_at, created_at, updated_at, - source_type, source_id, source_name, - field_id, field_label, field_type, field_group_id, field_group_label, - value_text, value_number, value_boolean, value_date, - metadata, language, user_id, tenant_id, submission_id - `, strings.Join(updates, ", "), argCount, argCount+1) + RETURNING `+feedbackRecordColumns, + strings.Join(updates, ", "), argCount, argCount+1) return query, args, true } @@ -415,7 +534,7 @@ func (r *FeedbackRecordsRepository) Update( return r.GetByID(ctx, id) } - var record models.FeedbackRecord + var record *models.FeedbackRecord err := withTenantWritePoolTx(ctx, r.db, nil, func(dbTx tenantWriteTx) error { tenantID, err := lockFeedbackRecordTenantShared(ctx, dbTx, id) @@ -423,13 +542,7 @@ func (r *FeedbackRecordsRepository) Update( return err } - err = dbTx.QueryRow(ctx, query, append(args, tenantID)...).Scan( - &record.ID, &record.CollectedAt, &record.CreatedAt, &record.UpdatedAt, - &record.SourceType, &record.SourceID, &record.SourceName, - &record.FieldID, &record.FieldLabel, &record.FieldType, &record.FieldGroupID, &record.FieldGroupLabel, - &record.ValueText, &record.ValueNumber, &record.ValueBoolean, &record.ValueDate, - &record.Metadata, &record.Language, &record.UserID, &record.TenantID, &record.SubmissionID, - ) + scanned, err := scanFeedbackRecord(dbTx.QueryRow(ctx, query, append(args, tenantID)...)) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return huberrors.NewNotFoundError("feedback record", "feedback record not found") @@ -438,13 +551,15 @@ func (r *FeedbackRecordsRepository) Update( return fmt.Errorf("failed to update feedback record: %w", err) } + record = scanned + return nil }) if err != nil { return nil, err } - return &record, nil + return record, nil } // Delete removes a feedback record. @@ -637,20 +752,12 @@ func (r *FeedbackRecordsRepository) fetchFeedbackRecords( records := []models.FeedbackRecord{} for rows.Next() { - var record models.FeedbackRecord - - err := rows.Scan( - &record.ID, &record.CollectedAt, &record.CreatedAt, &record.UpdatedAt, - &record.SourceType, &record.SourceID, &record.SourceName, - &record.FieldID, &record.FieldLabel, &record.FieldType, &record.FieldGroupID, &record.FieldGroupLabel, - &record.ValueText, &record.ValueNumber, &record.ValueBoolean, &record.ValueDate, - &record.Metadata, &record.Language, &record.UserID, &record.TenantID, &record.SubmissionID, - ) + record, err := scanFeedbackRecord(rows) if err != nil { return nil, fmt.Errorf("failed to scan feedback record: %w", err) } - records = append(records, record) + records = append(records, *record) } if err := rows.Err(); err != nil { diff --git a/internal/repository/taxonomy_repository.go b/internal/repository/taxonomy_repository.go index 13c77063..79b3545c 100644 --- a/internal/repository/taxonomy_repository.go +++ b/internal/repository/taxonomy_repository.go @@ -788,7 +788,8 @@ func (r *TaxonomyRepository) ListNodeRecords( fr.source_type, fr.source_id, fr.source_name, fr.field_id, fr.field_label, fr.field_type, fr.field_group_id, fr.field_group_label, fr.value_text, fr.value_number, fr.value_boolean, fr.value_date, - fr.metadata, fr.language, fr.user_id, fr.tenant_id, fr.submission_id + fr.metadata, fr.language, fr.user_id, fr.tenant_id, fr.submission_id, + fr.value_text_translated, fr.translation_lang_key FROM visible_nodes vn INNER JOIN taxonomy_runs tr ON tr.id = vn.run_id INNER JOIN taxonomy_cluster_memberships tcm ON tcm.run_id = vn.run_id AND tcm.cluster_id = vn.cluster_id @@ -1184,6 +1185,8 @@ func scanFeedbackRecord(row scanner) (*models.FeedbackRecord, error) { &record.UserID, &record.TenantID, &record.SubmissionID, + &record.ValueTextTranslated, + &record.TranslationLangKey, ); err != nil { return nil, fmt.Errorf("scan feedback record: %w", err) } diff --git a/internal/service/embedding_job_args.go b/internal/service/embedding_job_args.go index 21c81127..4d708e10 100644 --- a/internal/service/embedding_job_args.go +++ b/internal/service/embedding_job_args.go @@ -1,11 +1,8 @@ package service import ( - "context" - "github.com/google/uuid" "github.com/riverqueue/river" - "github.com/riverqueue/river/rivertype" ) const ( @@ -14,11 +11,6 @@ const ( EmbeddingsQueueName = "embeddings" ) -// FeedbackEmbeddingInserter inserts embedding jobs (e.g. River client). Used by BackfillEmbeddings. -type FeedbackEmbeddingInserter interface { - Insert(ctx context.Context, args river.JobArgs, opts *river.InsertOpts) (*rivertype.JobInsertResult, error) -} - // FeedbackEmbeddingArgs is the job payload for generating and storing an embedding for one feedback record. // Used by EmbeddingProvider and the backfill flow to enqueue, and by FeedbackEmbeddingWorker to run. // Uniqueness is by (FeedbackRecordID, Model, ValueTextHash) so that edits within the uniqueness window diff --git a/internal/service/embedding_provider.go b/internal/service/embedding_provider.go index 8e57390c..0364089e 100644 --- a/internal/service/embedding_provider.go +++ b/internal/service/embedding_provider.go @@ -21,7 +21,7 @@ import ( // when the event is FeedbackRecordCreated (with non-empty value_text) or FeedbackRecordUpdated // (with value_text in ChangedFields, including when value_text is now empty so the worker can clear). type EmbeddingProvider struct { - inserter FeedbackEmbeddingInserter + inserter RiverJobInserter apiKey string model string queueName string @@ -35,7 +35,7 @@ type EmbeddingProvider struct { // docPrefix is the prefix for document text (from EmbeddingPrefixForProvider); use "" for OpenAI/Google. // metrics may be nil when metrics are disabled. func NewEmbeddingProvider( - inserter FeedbackEmbeddingInserter, + inserter RiverJobInserter, apiKey string, model string, queueName string, diff --git a/internal/service/enrichment_settings_listener.go b/internal/service/enrichment_settings_listener.go new file mode 100644 index 00000000..2e57d13d --- /dev/null +++ b/internal/service/enrichment_settings_listener.go @@ -0,0 +1,73 @@ +package service + +import ( + "context" + "fmt" + "log/slog" + + "github.com/riverqueue/river" +) + +// enrichmentBackfillHandler enqueues the per-tenant backfill that one enrichment needs when +// its triggering setting changes. +type enrichmentBackfillHandler func(ctx context.Context, tenantID string) error + +// EnrichmentSettingsListener implements SettingsChangeListener by dispatching each changed +// setting key to the enrichment backfill it triggers. The dispatch table is built from what +// the deployment has enabled (e.g. translation), so a disabled enrichment registers no +// handler and an unknown/irrelevant key is ignored. +// +// It deliberately does not use the webhook MessagePublisher: this is an internal, +// best-effort side-effect, not a customer-facing event. Enqueue errors are logged and +// swallowed — the global backfill command remains the guaranteed recovery path. +// +// Adding a future setting is one map entry (e.g. a "sentiment_enabled" → sentiment backfill +// handler); TenantSettingsService does not change. +type EnrichmentSettingsListener struct { + handlers map[string]enrichmentBackfillHandler +} + +// NewTranslationSettingsListener builds a listener that, on a target_language change, +// enqueues a per-tenant translation backfill job (TenantTranslationBackfillArgs) via +// inserter. queueName/maxAttempts configure that fan-out job. +func NewTranslationSettingsListener( + inserter RiverJobInserter, queueName string, maxAttempts int, +) *EnrichmentSettingsListener { + return &EnrichmentSettingsListener{ + handlers: map[string]enrichmentBackfillHandler{ + settingKeyTargetLanguage: func(ctx context.Context, tenantID string) error { + _, err := inserter.Insert(ctx, TenantTranslationBackfillArgs{TenantID: tenantID}, &river.InsertOpts{ + Queue: queueName, + MaxAttempts: maxAttempts, + // Unique by TenantID across in-flight states (no ByPeriod): coalesce + // rapid changes into one backfill, but let a change after the previous + // one completed re-trigger. + UniqueOpts: river.UniqueOpts{ByArgs: true}, + }) + if err != nil { + return fmt.Errorf("enqueue tenant translation backfill: %w", err) + } + + return nil + }, + }, + } +} + +// OnSettingsChanged dispatches each changed key to its enrichment backfill handler, if one +// is registered. Errors are logged and swallowed (the settings write already succeeded). +func (l *EnrichmentSettingsListener) OnSettingsChanged(ctx context.Context, tenantID string, changedKeys []string) { + for _, key := range changedKeys { + handler, ok := l.handlers[key] + if !ok { + continue + } + + if err := handler(ctx, tenantID); err != nil { + slog.Error("enrichment settings listener: backfill enqueue failed", + "tenant_id", tenantID, "setting", key, "error", err) + } + } +} + +var _ SettingsChangeListener = (*EnrichmentSettingsListener)(nil) diff --git a/internal/service/enrichment_settings_listener_test.go b/internal/service/enrichment_settings_listener_test.go new file mode 100644 index 00000000..9694c982 --- /dev/null +++ b/internal/service/enrichment_settings_listener_test.go @@ -0,0 +1,77 @@ +package service + +import ( + "context" + "errors" + "testing" + + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" +) + +// recordingInserter records every job inserted (args + opts) and returns a fixed error. +type recordingInserter struct { + args []river.JobArgs + opts []*river.InsertOpts + err error +} + +func (r *recordingInserter) Insert( + _ context.Context, args river.JobArgs, opts *river.InsertOpts, +) (*rivertype.JobInsertResult, error) { + r.args = append(r.args, args) + r.opts = append(r.opts, opts) + + return &rivertype.JobInsertResult{}, r.err +} + +func TestEnrichmentSettingsListener_DispatchesTargetLanguage(t *testing.T) { + inserter := &recordingInserter{} + listener := NewTranslationSettingsListener(inserter, TranslationBackfillsQueueName, 3) + + listener.OnSettingsChanged(context.Background(), "org-1", []string{settingKeyTargetLanguage}) + + if len(inserter.args) != 1 { + t.Fatalf("enqueued %d jobs, want 1", len(inserter.args)) + } + + job, ok := inserter.args[0].(TenantTranslationBackfillArgs) + if !ok { + t.Fatalf("enqueued args type = %T, want TenantTranslationBackfillArgs", inserter.args[0]) + } + + if job.TenantID != "org-1" { + t.Fatalf("job tenant = %q, want org-1", job.TenantID) + } + + if inserter.opts[0].Queue != TranslationBackfillsQueueName { + t.Fatalf("queue = %q, want %q", inserter.opts[0].Queue, TranslationBackfillsQueueName) + } + + if !inserter.opts[0].UniqueOpts.ByArgs { + t.Fatal("want ByArgs uniqueness (one in-flight backfill per tenant)") + } +} + +func TestEnrichmentSettingsListener_IgnoresUnknownKey(t *testing.T) { + inserter := &recordingInserter{} + listener := NewTranslationSettingsListener(inserter, TranslationBackfillsQueueName, 3) + + listener.OnSettingsChanged(context.Background(), "org-1", []string{"some_other_setting"}) + + if len(inserter.args) != 0 { + t.Fatalf("enqueued %d jobs for an unrelated key, want 0", len(inserter.args)) + } +} + +func TestEnrichmentSettingsListener_SwallowsEnqueueError(t *testing.T) { + inserter := &recordingInserter{err: errors.New("insert failed")} + listener := NewTranslationSettingsListener(inserter, TranslationBackfillsQueueName, 3) + + // Must neither panic nor propagate: the settings write already committed. + listener.OnSettingsChanged(context.Background(), "org-1", []string{settingKeyTargetLanguage}) + + if len(inserter.args) != 1 { + t.Fatalf("attempted %d inserts, want 1", len(inserter.args)) + } +} diff --git a/internal/service/feedback_records_service.go b/internal/service/feedback_records_service.go index 766ce98f..59881684 100644 --- a/internal/service/feedback_records_service.go +++ b/internal/service/feedback_records_service.go @@ -24,6 +24,11 @@ var ErrUserIDRequired = huberrors.NewValidationError("user_id", "user_id is requ // ErrEmbeddingBackfillNotConfigured is returned when BackfillEmbeddings is called without embedding inserter/queue. var ErrEmbeddingBackfillNotConfigured = errors.New("embedding backfill not configured") +// ErrTranslationLangKeyRequired is returned when a translation is set without a target +// locale key: a translation must record the locale it was produced in (clearing, where +// translated is nil, intentionally passes an empty key to null both columns). +var ErrTranslationLangKeyRequired = errors.New("translation lang key is required when translated text is set") + const uniqueByPeriodEmbedding = 24 * time.Hour // FeedbackRecordsRepository defines the interface for feedback records data access. @@ -36,6 +41,13 @@ type FeedbackRecordsRepository interface { cursorCollectedAt time.Time, cursorID uuid.UUID, ) ([]models.FeedbackRecord, bool, error) Update(ctx context.Context, id uuid.UUID, req *models.UpdateFeedbackRecordRequest) (*models.FeedbackRecord, error) + SetTranslation(ctx context.Context, feedbackRecordID uuid.UUID, translated *string, langKey string) error + ListTranslationBackfillTargets( + ctx context.Context, afterID uuid.UUID, limit int, + ) ([]models.TranslationBackfillTarget, error) + ListTranslationBackfillTargetsForTenant( + ctx context.Context, tenantID string, afterID uuid.UUID, limit int, + ) ([]models.TranslationBackfillTarget, error) Delete(ctx context.Context, id uuid.UUID) error DeleteByUser(ctx context.Context, filters *models.DeleteFeedbackRecordsByUserFilters) ([]models.DeletedFeedbackRecordsByTenant, error) } @@ -53,7 +65,7 @@ type FeedbackRecordsService struct { embeddingsRepo EmbeddingsRepository embeddingModel string publisher MessagePublisher - embeddingInserter FeedbackEmbeddingInserter + embeddingInserter RiverJobInserter embeddingQueueName string embeddingMaxAttempts int } @@ -68,7 +80,7 @@ func NewFeedbackRecordsService( embeddingsRepo EmbeddingsRepository, embeddingModel string, publisher MessagePublisher, - embeddingInserter FeedbackEmbeddingInserter, + embeddingInserter RiverJobInserter, embeddingQueueName string, embeddingMaxAttempts int, ) *FeedbackRecordsService { @@ -85,7 +97,7 @@ func NewFeedbackRecordsService( // SetEmbeddingInserter sets the River inserter for embedding jobs (e.g. after River client is created). // This allows a single service instance to be used by both handlers and the embedding worker. -func (s *FeedbackRecordsService) SetEmbeddingInserter(inserter FeedbackEmbeddingInserter) { +func (s *FeedbackRecordsService) SetEmbeddingInserter(inserter RiverJobInserter) { s.embeddingInserter = inserter } @@ -123,6 +135,25 @@ func (s *FeedbackRecordsService) GetFeedbackRecord(ctx context.Context, id uuid. return record, nil } +// SetTranslation persists the translated value_text and the target locale key for a +// feedback record. It is the accessor the translation worker uses; the write is +// tenant-write-locked in the repository and publishes no event (no enrichment loop). +func (s *FeedbackRecordsService) SetTranslation( + ctx context.Context, feedbackRecordID uuid.UUID, translated *string, langKey string, +) error { + // A translation must carry the locale it was produced in; reject an inconsistent + // (translated, "") pair. Clearing (translated == nil) intentionally passes "". + if translated != nil && strings.TrimSpace(langKey) == "" { + return ErrTranslationLangKeyRequired + } + + if err := s.repo.SetTranslation(ctx, feedbackRecordID, translated, langKey); err != nil { + return fmt.Errorf("set feedback record translation: %w", err) + } + + return nil +} + // ListFeedbackRecords retrieves a list of feedback records with optional filters. // Uses cursor-based pagination: omit cursor for first page, use next_cursor for subsequent pages. func (s *FeedbackRecordsService) ListFeedbackRecords( @@ -329,3 +360,123 @@ func (s *FeedbackRecordsService) BackfillEmbeddings(ctx context.Context, model s return enqueued, nil } + +// BackfillTranslations enqueues a translation job for every feedback record that needs +// (re)translation to its tenant's configured target language (text records with non-empty +// value_text whose translation is missing or stale). The worker re-resolves the record at +// run time; the "backfill" hash marks these jobs distinct from event-driven ones. The +// inserter, queue, and max attempts come from the caller (a one-off command), so the shared +// service holds no backfill-only dependency. Returns the number of jobs enqueued. +// translationBackfillPageSize bounds how many stale records a backfill lists and enqueues +// per keyset page, so neither the global nor the per-tenant backfill materializes the full +// result set in memory. +const translationBackfillPageSize = 500 + +// BackfillTranslations enqueues a translation job for every feedback record (across all +// tenants) that needs (re)translation, streaming the targets in keyset pages. Used by the +// one-off global backfill command. Returns the number of jobs enqueued. +func (s *FeedbackRecordsService) BackfillTranslations( + ctx context.Context, inserter RiverJobInserter, queueName string, maxAttempts int, +) (int, error) { + return s.backfillTranslationsPaged(ctx, inserter, translationBackfillInsertOpts(queueName, maxAttempts), + func(afterID uuid.UUID) ([]models.TranslationBackfillTarget, error) { + targets, err := s.repo.ListTranslationBackfillTargets(ctx, afterID, translationBackfillPageSize) + if err != nil { + return nil, fmt.Errorf("list translation backfill targets: %w", err) + } + + return targets, nil + }) +} + +// BackfillTranslationsForTenant enqueues a translation job for every record of a single +// tenant that needs (re)translation, streaming in keyset pages so a large tenant is never +// fully materialized. It is the bulk work behind a settings-change re-translation +// (TenantTranslationBackfillArgs). Returns the number of jobs enqueued. +func (s *FeedbackRecordsService) BackfillTranslationsForTenant( + ctx context.Context, inserter RiverJobInserter, queueName string, maxAttempts int, tenantID string, +) (int, error) { + return s.backfillTranslationsPaged(ctx, inserter, translationBackfillInsertOpts(queueName, maxAttempts), + func(afterID uuid.UUID) ([]models.TranslationBackfillTarget, error) { + targets, err := s.repo.ListTranslationBackfillTargetsForTenant(ctx, tenantID, afterID, translationBackfillPageSize) + if err != nil { + return nil, fmt.Errorf("list translation backfill targets for tenant %s: %w", tenantID, err) + } + + return targets, nil + }) +} + +// backfillTranslationsPaged enqueues a translation job for every target produced by +// fetchPage, streaming in keyset pages (so the full set is never materialized) and stopping +// on the first short page. Advancing the cursor by the last id seen means even a +// fully-deduped page cannot livelock the loop. +func (s *FeedbackRecordsService) backfillTranslationsPaged( + ctx context.Context, + inserter RiverJobInserter, + opts *river.InsertOpts, + fetchPage func(afterID uuid.UUID) ([]models.TranslationBackfillTarget, error), +) (int, error) { + enqueued := 0 + afterID := uuid.Nil + + for { + targets, err := fetchPage(afterID) + if err != nil { + return enqueued, err + } + + if len(targets) == 0 { + break + } + + inserted, err := enqueueTranslationBackfillJobs(ctx, inserter, opts, targets) + enqueued += inserted + + if err != nil { + return enqueued, err + } + + afterID = targets[len(targets)-1].FeedbackRecordID + + if len(targets) < translationBackfillPageSize { + break + } + } + + return enqueued, nil +} + +// translationBackfillInsertOpts is the shared River insert config for backfill translation +// jobs: per-record dedup by (record, target, hash) within the 24h window. +func translationBackfillInsertOpts(queueName string, maxAttempts int) *river.InsertOpts { + return &river.InsertOpts{ + Queue: queueName, + MaxAttempts: maxAttempts, + UniqueOpts: river.UniqueOpts{ByArgs: true, ByPeriod: uniqueByPeriodTranslation}, + } +} + +// enqueueTranslationBackfillJobs inserts one FeedbackTranslationArgs per target. The +// "backfill" hash marks these jobs distinct from event-driven ones. It returns the count +// enqueued and stops on the first insert error. +func enqueueTranslationBackfillJobs( + ctx context.Context, inserter RiverJobInserter, opts *river.InsertOpts, + targets []models.TranslationBackfillTarget, +) (int, error) { + enqueued := 0 + + for _, target := range targets { + if _, err := inserter.Insert(ctx, FeedbackTranslationArgs{ + FeedbackRecordID: target.FeedbackRecordID, + TargetLang: target.TargetLang, + ValueTextHash: "backfill", + }, opts); err != nil { + return enqueued, fmt.Errorf("enqueue translation job for %s: %w", target.FeedbackRecordID, err) + } + + enqueued++ + } + + return enqueued, nil +} diff --git a/internal/service/feedback_records_service_test.go b/internal/service/feedback_records_service_test.go index 6e0b2821..ffa3b173 100644 --- a/internal/service/feedback_records_service_test.go +++ b/internal/service/feedback_records_service_test.go @@ -15,11 +15,15 @@ import ( ) type mockFeedbackRecordsRepo struct { - record *models.FeedbackRecord - createReq *models.CreateFeedbackRecordRequest - deleteByUserGroups []models.DeletedFeedbackRecordsByTenant - deletedID uuid.UUID - deleteByUserFilters *models.DeleteFeedbackRecordsByUserFilters + record *models.FeedbackRecord + createReq *models.CreateFeedbackRecordRequest + deleteByUserGroups []models.DeletedFeedbackRecordsByTenant + deletedID uuid.UUID + deleteByUserFilters *models.DeleteFeedbackRecordsByUserFilters + translationBackfillTargets []models.TranslationBackfillTarget + translationBackfillErr error + tenantBackfillTargets []models.TranslationBackfillTarget + tenantBackfillErr error } func (m *mockFeedbackRecordsRepo) Create( @@ -57,6 +61,43 @@ func (m *mockFeedbackRecordsRepo) Update( return nil, errors.New("not implemented") } +func (m *mockFeedbackRecordsRepo) SetTranslation( + _ context.Context, _ uuid.UUID, _ *string, _ string, +) error { + return nil +} + +func (m *mockFeedbackRecordsRepo) ListTranslationBackfillTargets( + _ context.Context, afterID uuid.UUID, _ int, +) ([]models.TranslationBackfillTarget, error) { + if m.translationBackfillErr != nil { + return nil, m.translationBackfillErr + } + + // First page (afterID == Nil) returns the configured set; later pages are empty. + if afterID != uuid.Nil { + return nil, nil + } + + return m.translationBackfillTargets, nil +} + +func (m *mockFeedbackRecordsRepo) ListTranslationBackfillTargetsForTenant( + _ context.Context, _ string, afterID uuid.UUID, _ int, +) ([]models.TranslationBackfillTarget, error) { + if m.tenantBackfillErr != nil { + return nil, m.tenantBackfillErr + } + + // First page (afterID == Nil) returns the configured set; later pages are empty, so + // the keyset loop terminates after one short page. + if afterID != uuid.Nil { + return nil, nil + } + + return m.tenantBackfillTargets, nil +} + func (m *mockFeedbackRecordsRepo) Delete(_ context.Context, id uuid.UUID) error { m.deletedID = id @@ -278,6 +319,96 @@ func TestFeedbackRecordsService_DeleteFeedbackRecordsByUser_RequiresUserID(t *te } } +func TestFeedbackRecordsService_SetTranslation_RequiresLangKey(t *testing.T) { + svc := NewFeedbackRecordsService(&mockFeedbackRecordsRepo{}, nil, "", nil, nil, "", 0) + translated := "Hallo" + + // A translation with a blank lang key is rejected before reaching the repo. + if err := svc.SetTranslation(context.Background(), uuid.New(), &translated, " "); !errors.Is(err, ErrTranslationLangKeyRequired) { + t.Fatalf("err = %v, want ErrTranslationLangKeyRequired", err) + } + + // Clearing (nil translation) with an empty key is allowed. + if err := svc.SetTranslation(context.Background(), uuid.New(), nil, ""); err != nil { + t.Fatalf("clear err = %v, want nil", err) + } +} + +func TestFeedbackRecordsService_BackfillTranslationsForTenant(t *testing.T) { + firstID := uuid.New() + secondID := uuid.New() + repo := &mockFeedbackRecordsRepo{ + tenantBackfillTargets: []models.TranslationBackfillTarget{ + {FeedbackRecordID: firstID, TargetLang: "de-DE"}, + {FeedbackRecordID: secondID, TargetLang: "de-DE"}, + }, + } + inserter := &mockTranslationInserter{} + svc := NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + enqueued, err := svc.BackfillTranslationsForTenant(context.Background(), inserter, TranslationsQueueName, 3, "org-1") + if err != nil { + t.Fatalf("BackfillTranslationsForTenant() error = %v", err) + } + + if enqueued != 2 || len(inserter.calls) != 2 { + t.Fatalf("enqueued=%d calls=%d, want 2/2", enqueued, len(inserter.calls)) + } + + if inserter.calls[0].FeedbackRecordID != firstID || + inserter.calls[0].TargetLang != "de-DE" || inserter.calls[0].ValueTextHash != "backfill" { + t.Fatalf("first job = %+v, want firstID/de-DE/backfill", inserter.calls[0]) + } +} + +func TestFeedbackRecordsService_BackfillTranslationsForTenant_RepoError(t *testing.T) { + repo := &mockFeedbackRecordsRepo{tenantBackfillErr: errors.New("db down")} + svc := NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + _, err := svc.BackfillTranslationsForTenant( + context.Background(), &mockTranslationInserter{}, TranslationsQueueName, 3, "org-1") + if err == nil { + t.Fatal("BackfillTranslationsForTenant() = nil error, want repo error") + } +} + +func TestFeedbackRecordsService_BackfillTranslations(t *testing.T) { + id1 := uuid.Must(uuid.NewV7()) + id2 := uuid.Must(uuid.NewV7()) + repo := &mockFeedbackRecordsRepo{ + translationBackfillTargets: []models.TranslationBackfillTarget{ + {FeedbackRecordID: id1, TargetLang: "de-DE"}, + {FeedbackRecordID: id2, TargetLang: "fr-FR"}, + }, + } + svc := NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + inserter := &mockTranslationInserter{} + + enqueued, err := svc.BackfillTranslations(context.Background(), inserter, TranslationsQueueName, 3) + if err != nil { + t.Fatalf("BackfillTranslations() error = %v", err) + } + + if enqueued != 2 || len(inserter.calls) != 2 { + t.Fatalf("enqueued = %d, inserter calls = %d, want 2 and 2", enqueued, len(inserter.calls)) + } + + first := inserter.calls[0] + if first.FeedbackRecordID != id1 || first.TargetLang != "de-DE" || first.ValueTextHash != "backfill" { + t.Fatalf("first job = %+v, want {%s de-DE backfill}", first, id1) + } +} + +func TestFeedbackRecordsService_BackfillTranslations_RepoError(t *testing.T) { + repo := &mockFeedbackRecordsRepo{translationBackfillErr: errors.New("boom")} + svc := NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + _, err := svc.BackfillTranslations(context.Background(), &mockTranslationInserter{}, TranslationsQueueName, 3) + if err == nil { + t.Fatal("BackfillTranslations() = nil, want the repo error propagated") + } +} + func assertDeletedEventDataAt( t *testing.T, publisher *capturePublisher, diff --git a/internal/service/job_inserter.go b/internal/service/job_inserter.go new file mode 100644 index 00000000..8c56423d --- /dev/null +++ b/internal/service/job_inserter.go @@ -0,0 +1,15 @@ +package service + +import ( + "context" + + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" +) + +// RiverJobInserter inserts a single River job. It is the shared seam the enrichment +// providers (embedding, translation) and their backfill flows use to enqueue work; +// satisfied by the River client. +type RiverJobInserter interface { + Insert(ctx context.Context, args river.JobArgs, opts *river.InsertOpts) (*rivertype.JobInsertResult, error) +} diff --git a/internal/service/settings_change_listener.go b/internal/service/settings_change_listener.go new file mode 100644 index 00000000..ea8f9d6d --- /dev/null +++ b/internal/service/settings_change_listener.go @@ -0,0 +1,15 @@ +package service + +import "context" + +// SettingsChangeListener is notified after a tenant's settings are successfully written, +// with the setting keys the write touched. It lets enrichment side-effects (e.g. +// re-translation) react to a settings change without TenantSettingsService depending on any +// enrichment concern — the service depends only on this translation-free port. +// +// Implementations do the reaction themselves (a fast, durable enqueue) and own their error +// handling: the method returns nothing because the settings write has already committed and +// the side-effect must never fail it. +type SettingsChangeListener interface { + OnSettingsChanged(ctx context.Context, tenantID string, changedKeys []string) +} diff --git a/internal/service/tenant_settings_cache.go b/internal/service/tenant_settings_cache.go new file mode 100644 index 00000000..7c295bb1 --- /dev/null +++ b/internal/service/tenant_settings_cache.go @@ -0,0 +1,82 @@ +package service + +import ( + "context" + "fmt" + "time" + + "github.com/hashicorp/golang-lru/v2/expirable" + + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/observability" +) + +// cacheNameTenantSettings is the bounded-cardinality metrics label for the +// tenant-settings cache (registered in observability.allowedCacheNames). +const cacheNameTenantSettings = "tenant_settings" + +// TenantSettingsReader is the read surface the cache wraps. *TenantSettingsService +// satisfies it, so the cache is a drop-in for any consumer that only needs reads +// (the translation enqueue gate and worker). +type TenantSettingsReader interface { + GetSettings(ctx context.Context, tenantID string) (*models.TenantSettings, error) +} + +// CachedTenantSettings wraps a TenantSettingsReader with a per-process, +// size-bounded, TTL-expiring LRU cache. Feedback-record creation is high volume and +// the translation enqueue gate must resolve a tenant's target language per event, +// so caching avoids a tenant_settings read on every feedback event. Staleness is +// bounded by the TTL; because the worker persists the target it actually used, a +// changed target self-corrects on the next write. Safe for concurrent use +// (expirable.LRU is internally locked). +type CachedTenantSettings struct { + delegate TenantSettingsReader + cache *expirable.LRU[string, *models.TenantSettings] + metrics observability.CacheMetrics // nil when metrics are disabled +} + +// NewCachedTenantSettings wraps delegate with an LRU of at most size entries, each +// expiring after ttl. A non-positive size or ttl disables caching (every read hits +// the delegate), keeping small deployments and tests simple. +func NewCachedTenantSettings( + delegate TenantSettingsReader, size int, ttl time.Duration, metrics observability.CacheMetrics, +) *CachedTenantSettings { + cached := &CachedTenantSettings{delegate: delegate, metrics: metrics} + if size > 0 && ttl > 0 { + cached.cache = expirable.NewLRU[string, *models.TenantSettings](size, nil, ttl) + } + + return cached +} + +// GetSettings returns the tenant's settings, serving a fresh cached value when +// present and otherwise loading from the delegate and caching the result. Errors +// are never cached. +func (c *CachedTenantSettings) GetSettings( + ctx context.Context, tenantID string, +) (*models.TenantSettings, error) { + if c.cache != nil { + if settings, ok := c.cache.Get(tenantID); ok { + if c.metrics != nil { + c.metrics.RecordHit(ctx, cacheNameTenantSettings) + } + + return settings, nil + } + + if c.metrics != nil { + c.metrics.RecordMiss(ctx, cacheNameTenantSettings) + } + } + + settings, err := c.delegate.GetSettings(ctx, tenantID) + if err != nil { + return nil, fmt.Errorf("load tenant settings: %w", err) + } + + if c.cache != nil { + c.cache.Add(tenantID, settings) + } + + return settings, nil +} diff --git a/internal/service/tenant_settings_cache_test.go b/internal/service/tenant_settings_cache_test.go new file mode 100644 index 00000000..4eb661f2 --- /dev/null +++ b/internal/service/tenant_settings_cache_test.go @@ -0,0 +1,82 @@ +package service + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/formbricks/hub/internal/models" +) + +// countingSettingsReader is a TenantSettingsReader that counts delegate calls. +type countingSettingsReader struct { + calls int + settings *models.TenantSettings + err error +} + +func (r *countingSettingsReader) GetSettings(_ context.Context, _ string) (*models.TenantSettings, error) { + r.calls++ + + if r.err != nil { + return nil, r.err + } + + return r.settings, nil +} + +func TestCachedTenantSettings_CachesWithinTTL(t *testing.T) { + reader := &countingSettingsReader{ + settings: &models.TenantSettings{ + TenantID: "org-1", + Settings: models.EnrichmentSettings{TargetLanguage: "de-DE"}, + }, + } + cached := NewCachedTenantSettings(reader, 16, time.Minute, nil) + + for range 3 { + got, err := cached.GetSettings(context.Background(), "org-1") + if err != nil { + t.Fatalf("GetSettings() error = %v", err) + } + + if got.Settings.TargetLanguage != "de-DE" { + t.Fatalf("target = %q, want de-DE", got.Settings.TargetLanguage) + } + } + + if reader.calls != 1 { + t.Fatalf("delegate calls = %d, want 1 (subsequent reads served from cache)", reader.calls) + } +} + +func TestCachedTenantSettings_DisabledBypassesCache(t *testing.T) { + reader := &countingSettingsReader{settings: &models.TenantSettings{TenantID: "org-1"}} + cached := NewCachedTenantSettings(reader, 0, time.Minute, nil) // size 0 disables caching + + for range 2 { + if _, err := cached.GetSettings(context.Background(), "org-1"); err != nil { + t.Fatalf("GetSettings() error = %v", err) + } + } + + if reader.calls != 2 { + t.Fatalf("delegate calls = %d, want 2 (cache disabled)", reader.calls) + } +} + +func TestCachedTenantSettings_DoesNotCacheErrors(t *testing.T) { + reader := &countingSettingsReader{err: errors.New("boom")} + cached := NewCachedTenantSettings(reader, 16, time.Minute, nil) + + for range 2 { + if _, err := cached.GetSettings(context.Background(), "org-1"); err == nil { + t.Fatal("GetSettings() expected error, got nil") + } + } + + if reader.calls != 2 { + t.Fatalf("delegate calls = %d, want 2 (errors must not be cached)", reader.calls) + } +} diff --git a/internal/service/tenant_settings_service.go b/internal/service/tenant_settings_service.go index e8822eed..84a2dc3b 100644 --- a/internal/service/tenant_settings_service.go +++ b/internal/service/tenant_settings_service.go @@ -37,7 +37,8 @@ const maxTargetLanguageLen = 35 // TenantSettingsService reads and writes tenant-scoped enrichment settings. It is // the accessor enrichment workflows will use to resolve a tenant's configuration. type TenantSettingsService struct { - repo TenantSettingsRepository + repo TenantSettingsRepository + listener SettingsChangeListener // optional; set via SetSettingsChangeListener } // NewTenantSettingsService creates a new tenant settings service. @@ -45,6 +46,14 @@ func NewTenantSettingsService(repo TenantSettingsRepository) *TenantSettingsServ return &TenantSettingsService{repo: repo} } +// SetSettingsChangeListener registers a listener notified after a successful settings write, +// used to trigger enrichment side-effects (e.g. a re-translation backfill on a +// target_language change). Optional; mirrors the post-construction injection of +// SetEmbeddingInserter. Nil means no side-effects fire. +func (s *TenantSettingsService) SetSettingsChangeListener(listener SettingsChangeListener) { + s.listener = listener +} + // GetSettings returns the tenant's enrichment settings. When the tenant has no // settings yet it returns a zero-value settings bag (target language unset) // rather than a not-found error: an unconfigured tenant is a valid state, and @@ -90,6 +99,9 @@ func (s *TenantSettingsService) UpdateSettings( return nil, fmt.Errorf("update tenant settings: %w", err) } + // PUT is a full replace, so every settable key is (re)written. + s.notifyChanged(ctx, normalizedTenantID, []string{settingKeyTargetLanguage}) + return settings, nil } @@ -108,11 +120,14 @@ func (s *TenantSettingsService) PatchSettings( } var ( - set models.EnrichmentSettings - removeKeys []string + set models.EnrichmentSettings + removeKeys []string + changedKeys []string ) if req.TargetLanguage.Present { + changedKeys = append(changedKeys, settingKeyTargetLanguage) + if req.TargetLanguage.Value == nil { // Explicit null: remove the setting (RFC 7396). removeKeys = append(removeKeys, settingKeyTargetLanguage) @@ -131,9 +146,22 @@ func (s *TenantSettingsService) PatchSettings( return nil, fmt.Errorf("patch tenant settings: %w", err) } + // Fire only for keys the patch actually touched (an omitted member does not trigger). + s.notifyChanged(ctx, normalizedTenantID, changedKeys) + return settings, nil } +// notifyChanged tells the registered listener which setting keys a write touched. No-op when +// no listener is set or nothing changed. +func (s *TenantSettingsService) notifyChanged(ctx context.Context, tenantID string, changedKeys []string) { + if s.listener == nil || len(changedKeys) == 0 { + return + } + + s.listener.OnSettingsChanged(ctx, tenantID, changedKeys) +} + // normalizeTargetLanguage trims and canonicalizes a BCP-47 locale (e.g. "en-us" // -> "en-US"). An empty value is allowed and normalizes to "" (target language // not configured) — this is the PUT full-replace semantics, where omitting the diff --git a/internal/service/tenant_settings_service_test.go b/internal/service/tenant_settings_service_test.go index baa500dc..39df09b3 100644 --- a/internal/service/tenant_settings_service_test.go +++ b/internal/service/tenant_settings_service_test.go @@ -78,6 +78,117 @@ func (m *mockTenantSettingsRepo) Patch( return &models.TenantSettings{TenantID: tenantID, Settings: settings}, nil } +type mockSettingsChangeListener struct { + tenantIDs []string + calls [][]string // changedKeys per call +} + +func (m *mockSettingsChangeListener) OnSettingsChanged(_ context.Context, tenantID string, changedKeys []string) { + m.tenantIDs = append(m.tenantIDs, tenantID) + m.calls = append(m.calls, changedKeys) +} + +func assertListenerFiredOnce(t *testing.T, listener *mockSettingsChangeListener, wantTenant, wantKey string) { + t.Helper() + + if len(listener.calls) != 1 { + t.Fatalf("listener fired %d times, want 1", len(listener.calls)) + } + + if listener.tenantIDs[0] != wantTenant { + t.Fatalf("listener tenant = %q, want %q", listener.tenantIDs[0], wantTenant) + } + + if len(listener.calls[0]) != 1 || listener.calls[0][0] != wantKey { + t.Fatalf("listener changedKeys = %v, want [%s]", listener.calls[0], wantKey) + } +} + +func TestTenantSettingsService_NotifiesListenerOnChange(t *testing.T) { + t.Run("PUT fires target_language", func(t *testing.T) { + repo := &mockTenantSettingsRepo{} + listener := &mockSettingsChangeListener{} + svc := NewTenantSettingsService(repo) + svc.SetSettingsChangeListener(listener) + + _, err := svc.UpdateSettings(context.Background(), "org-1", &models.UpdateTenantSettingsRequest{TargetLanguage: "de-DE"}) + if err != nil { + t.Fatalf("UpdateSettings() error = %v", err) + } + + assertListenerFiredOnce(t, listener, "org-1", "target_language") + }) + + t.Run("PATCH with a value fires", func(t *testing.T) { + repo := &mockTenantSettingsRepo{} + listener := &mockSettingsChangeListener{} + svc := NewTenantSettingsService(repo) + svc.SetSettingsChangeListener(listener) + + if _, err := svc.PatchSettings(context.Background(), "org-1", patchValue("fr-FR")); err != nil { + t.Fatalf("PatchSettings() error = %v", err) + } + + assertListenerFiredOnce(t, listener, "org-1", "target_language") + }) + + t.Run("PATCH null (removal) fires", func(t *testing.T) { + repo := &mockTenantSettingsRepo{} + listener := &mockSettingsChangeListener{} + svc := NewTenantSettingsService(repo) + svc.SetSettingsChangeListener(listener) + + req := &models.PatchTenantSettingsRequest{TargetLanguage: models.Optional[string]{Present: true, Value: nil}} + if _, err := svc.PatchSettings(context.Background(), "org-1", req); err != nil { + t.Fatalf("PatchSettings() error = %v", err) + } + + assertListenerFiredOnce(t, listener, "org-1", "target_language") + }) + + t.Run("PATCH omitted does not fire", func(t *testing.T) { + repo := &mockTenantSettingsRepo{} + listener := &mockSettingsChangeListener{} + svc := NewTenantSettingsService(repo) + svc.SetSettingsChangeListener(listener) + + if _, err := svc.PatchSettings(context.Background(), "org-1", &models.PatchTenantSettingsRequest{}); err != nil { + t.Fatalf("PatchSettings() error = %v", err) + } + + if len(listener.calls) != 0 { + t.Fatalf("listener fired %d times for a no-op PATCH, want 0", len(listener.calls)) + } + }) + + t.Run("does not fire when the write fails", func(t *testing.T) { + repo := &mockTenantSettingsRepo{upsertErr: errors.New("boom")} + listener := &mockSettingsChangeListener{} + svc := NewTenantSettingsService(repo) + svc.SetSettingsChangeListener(listener) + + if _, err := svc.UpdateSettings( + context.Background(), "org-1", &models.UpdateTenantSettingsRequest{TargetLanguage: "de-DE"}, + ); err == nil { + t.Fatal("UpdateSettings() = nil error, want repo error") + } + + if len(listener.calls) != 0 { + t.Fatalf("listener fired %d times despite a failed write, want 0", len(listener.calls)) + } + }) + + t.Run("no listener configured is safe", func(t *testing.T) { + svc := NewTenantSettingsService(&mockTenantSettingsRepo{}) + + if _, err := svc.UpdateSettings( + context.Background(), "org-1", &models.UpdateTenantSettingsRequest{TargetLanguage: "de-DE"}, + ); err != nil { + t.Fatalf("UpdateSettings() error = %v", err) + } + }) +} + func TestTenantSettingsService_GetSettings_NoRowReturnsDefaults(t *testing.T) { repo := &mockTenantSettingsRepo{getResult: nil} svc := NewTenantSettingsService(repo) diff --git a/internal/service/tenant_translation_backfill_job_args.go b/internal/service/tenant_translation_backfill_job_args.go new file mode 100644 index 00000000..e2bf3750 --- /dev/null +++ b/internal/service/tenant_translation_backfill_job_args.go @@ -0,0 +1,30 @@ +package service + +import "github.com/riverqueue/river" + +const ( + tenantTranslationBackfillKind = "tenant_translation_backfill" + // TranslationBackfillsQueueName is the River queue for per-tenant translation + // backfill fan-out jobs. It is kept separate from TranslationsQueueName so a large + // fan-out does not starve live per-record translation throughput. + TranslationBackfillsQueueName = "translation_backfills" +) + +// TenantTranslationBackfillArgs fans out a re-translation backfill for one tenant: the +// worker lists the tenant's stale text records and enqueues a FeedbackTranslationArgs +// job for each. It is enqueued when a tenant's translation-relevant settings change +// (today: target_language). +// +// Uniqueness is by TenantID across the default (in-flight) states — the enqueue site +// sets ByArgs without a ByPeriod — so rapid repeated settings changes collapse to a +// single in-flight backfill, while a change after the previous backfill has completed +// re-triggers. The worker resolves the tenant's current target at run time, so a +// coalesced backfill always targets the latest configured language. +type TenantTranslationBackfillArgs struct { + TenantID string `json:"tenant_id" river:"unique"` +} + +// Kind returns the River job kind. +func (TenantTranslationBackfillArgs) Kind() string { return tenantTranslationBackfillKind } + +var _ river.JobArgs = TenantTranslationBackfillArgs{} diff --git a/internal/service/translation_client.go b/internal/service/translation_client.go new file mode 100644 index 00000000..072f5bcd --- /dev/null +++ b/internal/service/translation_client.go @@ -0,0 +1,49 @@ +package service + +import ( + "context" + "strings" + + "golang.org/x/text/language" + "golang.org/x/text/language/display" +) + +// TranslateRequest is the input to a single translation. SourceLang and TargetLang +// are BCP-47 tags: TargetLang comes from the tenant's settings; SourceLang from the +// feedback record's language and may be empty when the source language is unknown. +type TranslateRequest struct { + Text string + SourceLang string + TargetLang string +} + +// TranslationClient translates TranslateRequest.Text from SourceLang into TargetLang +// and returns the translated text. Implementations call an LLM provider (OpenAI or +// Google); the factory selects one from configuration. It mirrors the +// EmbeddingClient seam so the worker depends on the interface, not a provider. +type TranslationClient interface { + Translate(ctx context.Context, req TranslateRequest) (string, error) +} + +// languageDisplayName returns the English display name of a BCP-47 language tag +// (e.g. "de-DE" -> "German") for the translation prompt, mirroring how Formbricks +// passes human-readable language names rather than raw codes to the model. It +// returns "" for an empty tag (an unknown source language) and falls back to the +// trimmed raw tag when it cannot be parsed or named. +func languageDisplayName(tag string) string { + trimmed := strings.TrimSpace(tag) + if trimmed == "" { + return "" + } + + parsed, err := language.Parse(trimmed) + if err != nil { + return trimmed + } + + if name := display.English.Languages().Name(parsed); name != "" { + return name + } + + return trimmed +} diff --git a/internal/service/translation_client_factory.go b/internal/service/translation_client_factory.go new file mode 100644 index 00000000..d6074a39 --- /dev/null +++ b/internal/service/translation_client_factory.go @@ -0,0 +1,195 @@ +package service + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/formbricks/hub/internal/googleai" + "github.com/formbricks/hub/internal/openai" +) + +// Translation provider names for NewTranslationClient (same identifiers as the +// embedding providers; translation reuses the OpenAI and Google SDK wrappers). +const ( + TranslationProviderOpenAI = "openai" + TranslationProviderGoogle = "google" + TranslationProviderGoogleGemini = "google-gemini" + + translationProviderGoogleVertexLegacy = "google-vertex" +) + +var ( + // ErrTranslationConfigInvalid is returned when the translation provider is unsupported. + ErrTranslationConfigInvalid = errors.New("translation config invalid") + // ErrTranslationProviderAPIKey is returned when an API-key-based provider is configured without a key. + ErrTranslationProviderAPIKey = errors.New("TRANSLATION_PROVIDER_API_KEY is required for this provider") + // ErrTranslationBaseURLUnsupported is returned when a custom base URL is configured for a non-openai provider. + ErrTranslationBaseURLUnsupported = errors.New("TRANSLATION_BASE_URL is only supported for openai") + // ErrTranslationGoogleGeminiConfig is returned when google-gemini is configured without project or location. + ErrTranslationGoogleGeminiConfig = errors.New( + "google-gemini requires TRANSLATION_GOOGLE_CLOUD_PROJECT and TRANSLATION_GOOGLE_CLOUD_LOCATION") +) + +// rawTranslator is the low-level provider call (system prompt + user text -> text), +// satisfied by *openai.Client and *googleai.Client. +type rawTranslator interface { + Translate(ctx context.Context, systemPrompt, userText string) (string, error) +} + +// promptTranslationClient adapts a rawTranslator to TranslationClient by building +// the translation prompt from the request (the provider call stays prompt-agnostic). +type promptTranslationClient struct { + raw rawTranslator +} + +// Translate builds the prompt and delegates to the provider. +func (c promptTranslationClient) Translate(ctx context.Context, req TranslateRequest) (string, error) { + systemPrompt, userText := buildTranslationPrompt(req) + + translated, err := c.raw.Translate(ctx, systemPrompt, userText) + if err != nil { + return "", fmt.Errorf("translate: %w", err) + } + + return translated, nil +} + +// buildTranslationPrompt renders the system prompt and user text for a request. It +// mirrors Formbricks' "professional translator" instruction, using human-readable +// language names and preserving placeholders/markup. When the source language is +// unknown it asks the model to translate from the original language. +func buildTranslationPrompt(req TranslateRequest) (systemPrompt, userText string) { + target := languageDisplayName(req.TargetLang) + if target == "" { + target = strings.TrimSpace(req.TargetLang) + } + + from := "its original language" + if source := languageDisplayName(req.SourceLang); source != "" { + from = source + } + + systemPrompt = fmt.Sprintf( + "You are a professional translator. Translate the user's message from %s into %s. "+ + "Preserve the original meaning, tone, and formatting, and keep any {{variable}} placeholders "+ + "and HTML tags unchanged. Respond with only the translated text — no preamble, notes, or quotation marks.", + from, target, + ) + + return systemPrompt, req.Text +} + +// translationProviderEntry describes capabilities and construction for one provider. +type translationProviderEntry struct { + RequiresAPIKey bool + RequiresGoogleGeminiConfig bool + Factory func(context.Context, TranslationClientConfig) (TranslationClient, error) +} + +// translationProviderRegistry is the single source of truth for provider capabilities and client creation. +var translationProviderRegistry = map[string]translationProviderEntry{ + TranslationProviderOpenAI: { + RequiresAPIKey: true, + Factory: openAITranslationFactory, + }, + TranslationProviderGoogle: { + RequiresAPIKey: true, + Factory: googleTranslationFactory, + }, + TranslationProviderGoogleGemini: { + RequiresGoogleGeminiConfig: true, + Factory: googleGeminiTranslationFactory, + }, +} + +func openAITranslationFactory(_ context.Context, cfg TranslationClientConfig) (TranslationClient, error) { + raw := openai.NewClient(cfg.ProviderAPIKey, + openai.WithModel(cfg.Model), + openai.WithBaseURL(cfg.BaseURL), + ) + + return promptTranslationClient{raw: raw}, nil +} + +func googleTranslationFactory(ctx context.Context, cfg TranslationClientConfig) (TranslationClient, error) { + raw, err := googleai.NewClient(ctx, cfg.ProviderAPIKey, googleai.WithModel(cfg.Model)) + if err != nil { + return nil, fmt.Errorf("create google translation client: %w", err) + } + + return promptTranslationClient{raw: raw}, nil +} + +func googleGeminiTranslationFactory(ctx context.Context, cfg TranslationClientConfig) (TranslationClient, error) { + raw, err := googleai.NewGoogleGeminiClient(ctx, cfg.GoogleCloudProject, cfg.GoogleCloudLocation, + googleai.WithModel(cfg.Model)) + if err != nil { + return nil, fmt.Errorf("create google-gemini translation client: %w", err) + } + + return promptTranslationClient{raw: raw}, nil +} + +// NormalizeTranslationProvider returns the canonical provider name (lowercase, trimmed). +func NormalizeTranslationProvider(provider string) string { + provider = strings.ToLower(strings.TrimSpace(provider)) + if provider == translationProviderGoogleVertexLegacy { + return TranslationProviderGoogleGemini + } + + return provider +} + +// TranslationClientConfig holds configuration for creating a translation client. +type TranslationClientConfig struct { + Provider string + ProviderAPIKey string // API key for openai/google providers; not logged or serialized + Model string + BaseURL string + GoogleCloudProject string + GoogleCloudLocation string +} + +// ValidateTranslationConfig checks provider support and provider-specific requirements. +// Use before creating a client or at startup to fail fast with a clear error. +func ValidateTranslationConfig(cfg TranslationClientConfig) error { + provider := NormalizeTranslationProvider(cfg.Provider) + + entry, ok := translationProviderRegistry[provider] + if !ok { + return fmt.Errorf("%w: unsupported provider %q", ErrTranslationConfigInvalid, provider) + } + + if entry.RequiresAPIKey && cfg.ProviderAPIKey == "" { + return fmt.Errorf("%w: %s", ErrTranslationProviderAPIKey, provider) + } + + if cfg.BaseURL != "" && provider != TranslationProviderOpenAI { + return fmt.Errorf("%w: %s", ErrTranslationBaseURLUnsupported, provider) + } + + if entry.RequiresGoogleGeminiConfig && (cfg.GoogleCloudProject == "" || cfg.GoogleCloudLocation == "") { + return ErrTranslationGoogleGeminiConfig + } + + return nil +} + +// NewTranslationClient creates a TranslationClient for the given config. It validates +// provider-specific requirements via the registry, then calls the registry factory. +func NewTranslationClient(ctx context.Context, cfg TranslationClientConfig) (TranslationClient, error) { + provider := NormalizeTranslationProvider(cfg.Provider) + + entry, ok := translationProviderRegistry[provider] + if !ok { + return nil, fmt.Errorf("%w: unsupported provider %q", ErrTranslationConfigInvalid, provider) + } + + if err := ValidateTranslationConfig(cfg); err != nil { + return nil, err + } + + return entry.Factory(ctx, cfg) +} diff --git a/internal/service/translation_client_factory_test.go b/internal/service/translation_client_factory_test.go new file mode 100644 index 00000000..676f29a3 --- /dev/null +++ b/internal/service/translation_client_factory_test.go @@ -0,0 +1,84 @@ +package service + +import ( + "errors" + "strings" + "testing" +) + +func TestValidateTranslationConfig(t *testing.T) { + tests := []struct { + name string + cfg TranslationClientConfig + wantErr error + }{ + {name: "openai ok", cfg: TranslationClientConfig{Provider: "openai", ProviderAPIKey: "k", Model: "gpt-x"}}, + { + name: "openai with base url ok", + cfg: TranslationClientConfig{Provider: "openai", ProviderAPIKey: "k", BaseURL: "https://x"}, + }, + {name: "google ok", cfg: TranslationClientConfig{Provider: "google", ProviderAPIKey: "k", Model: "gemini-x"}}, + { + name: "google-gemini ok", + cfg: TranslationClientConfig{Provider: "google-gemini", GoogleCloudProject: "p", GoogleCloudLocation: "l"}, + }, + { + name: "google-vertex legacy normalizes", + cfg: TranslationClientConfig{Provider: "google-vertex", GoogleCloudProject: "p", GoogleCloudLocation: "l"}, + }, + { + name: "unsupported provider", + cfg: TranslationClientConfig{Provider: "anthropic"}, + wantErr: ErrTranslationConfigInvalid, + }, + {name: "openai missing key", cfg: TranslationClientConfig{Provider: "openai"}, wantErr: ErrTranslationProviderAPIKey}, + {name: "google missing key", cfg: TranslationClientConfig{Provider: "google"}, wantErr: ErrTranslationProviderAPIKey}, + { + name: "base url rejected for google", + cfg: TranslationClientConfig{Provider: "google", ProviderAPIKey: "k", BaseURL: "https://x"}, + wantErr: ErrTranslationBaseURLUnsupported, + }, + { + name: "gemini missing project/location", + cfg: TranslationClientConfig{Provider: "google-gemini"}, + wantErr: ErrTranslationGoogleGeminiConfig, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + err := ValidateTranslationConfig(testCase.cfg) + + switch { + case testCase.wantErr == nil && err != nil: + t.Fatalf("ValidateTranslationConfig() error = %v, want nil", err) + case testCase.wantErr != nil && !errors.Is(err, testCase.wantErr): + t.Fatalf("ValidateTranslationConfig() error = %v, want %v", err, testCase.wantErr) + } + }) + } +} + +func TestBuildTranslationPrompt(t *testing.T) { + t.Run("known source and target render language names", func(t *testing.T) { + systemPrompt, userText := buildTranslationPrompt( + TranslateRequest{Text: "Bonjour", SourceLang: "fr", TargetLang: "de-DE"}) + + if userText != "Bonjour" { + t.Fatalf("user text = %q, want Bonjour", userText) + } + + if !strings.Contains(systemPrompt, "from French into German") { + t.Fatalf("system prompt missing language names: %q", systemPrompt) + } + }) + + t.Run("unknown source falls back to original language", func(t *testing.T) { + systemPrompt, _ := buildTranslationPrompt( + TranslateRequest{Text: "x", SourceLang: "", TargetLang: "fr"}) + + if !strings.Contains(systemPrompt, "from its original language into French") { + t.Fatalf("system prompt missing original-language fallback: %q", systemPrompt) + } + }) +} diff --git a/internal/service/translation_client_test.go b/internal/service/translation_client_test.go new file mode 100644 index 00000000..9e291553 --- /dev/null +++ b/internal/service/translation_client_test.go @@ -0,0 +1,26 @@ +package service + +import "testing" + +func TestLanguageDisplayName(t *testing.T) { + tests := []struct { + name string + tag string + want string + }{ + {name: "locale maps to language name", tag: "de-DE", want: "German"}, + {name: "language only", tag: "fr", want: "French"}, + {name: "another language", tag: "ja", want: "Japanese"}, + {name: "empty is empty (unknown source language)", tag: "", want: ""}, + {name: "whitespace is empty", tag: " ", want: ""}, + {name: "unparseable falls back to the raw tag", tag: "@@@bogus", want: "@@@bogus"}, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + if got := languageDisplayName(testCase.tag); got != testCase.want { + t.Fatalf("languageDisplayName(%q) = %q, want %q", testCase.tag, got, testCase.want) + } + }) + } +} diff --git a/internal/service/translation_job_args.go b/internal/service/translation_job_args.go new file mode 100644 index 00000000..3a9dd4cf --- /dev/null +++ b/internal/service/translation_job_args.go @@ -0,0 +1,30 @@ +package service + +import ( + "github.com/google/uuid" + "github.com/riverqueue/river" +) + +const ( + feedbackTranslationKind = "feedback_translation" + // TranslationsQueueName is the River queue used for feedback translation jobs. + TranslationsQueueName = "translations" +) + +// FeedbackTranslationArgs is the job payload for translating one feedback record's +// value_text into the tenant's target language. Uniqueness is by (FeedbackRecordID, +// TargetLang, ValueTextHash): a value_text edit or a target-language change yields a +// new job, while identical content for the same target within the window is deduped. +type FeedbackTranslationArgs struct { + FeedbackRecordID uuid.UUID `json:"feedback_record_id" river:"unique"` + // TargetLang is the tenant's configured target language (BCP-47) at enqueue time. + TargetLang string `json:"target_lang" river:"unique"` + // ValueTextHash is a hash of the inputs that determine the translation — the + // normalized value_text and the source language — or "empty" when value_text is blank. + ValueTextHash string `json:"value_text_hash" river:"unique"` +} + +// Kind returns the River job kind. +func (FeedbackTranslationArgs) Kind() string { return feedbackTranslationKind } + +var _ river.JobArgs = FeedbackTranslationArgs{} diff --git a/internal/service/translation_provider.go b/internal/service/translation_provider.go new file mode 100644 index 00000000..ffc3cbfe --- /dev/null +++ b/internal/service/translation_provider.go @@ -0,0 +1,163 @@ +package service + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "log/slog" + "strings" + "time" + + "github.com/riverqueue/river" + "golang.org/x/text/unicode/norm" + + "github.com/formbricks/hub/internal/datatypes" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/observability" +) + +// uniqueByPeriodTranslation dedupes identical translation jobs (same record, target, +// and value_text) within this window, mirroring the embedding pipeline. +const uniqueByPeriodTranslation = 24 * time.Hour + +// TranslationProvider implements eventPublisher by enqueueing one translation job per +// eligible feedback record event: FeedbackRecordCreated with non-empty open text, or +// FeedbackRecordUpdated whose value_text changed. A job is only enqueued when the +// record is a text field with non-empty value_text and the tenant has a target +// language configured (read via the settings resolver). The worker resolves the +// remaining work; ingestion is never blocked. +type TranslationProvider struct { + inserter RiverJobInserter + resolver TenantSettingsReader + queueName string + maxAttempts int + metrics observability.TranslationMetrics +} + +// NewTranslationProvider creates a provider that enqueues feedback_translation jobs. +// metrics may be nil when metrics are disabled. +func NewTranslationProvider( + inserter RiverJobInserter, + resolver TenantSettingsReader, + queueName string, + maxAttempts int, + metrics observability.TranslationMetrics, +) *TranslationProvider { + return &TranslationProvider{ + inserter: inserter, + resolver: resolver, + queueName: queueName, + maxAttempts: maxAttempts, + metrics: metrics, + } +} + +// PublishEvent enqueues a feedback_translation job for an eligible create/update event. +// Failures are logged and swallowed so they never block ingestion. +func (p *TranslationProvider) PublishEvent(ctx context.Context, event Event) { + if event.Type == datatypes.FeedbackRecordUpdated { + // Re-translate when the text or its source language changes: translation + // output depends on both (unlike embeddings, which ignore source language). + if !contains(event.ChangedFields, "value_text") && !contains(event.ChangedFields, "language") { + return + } + } else if event.Type != datatypes.FeedbackRecordCreated { + return + } + + record, ok := event.Data.(*models.FeedbackRecord) + if !ok { + slog.Debug("translation: skip, event data is not *FeedbackRecord", "event_id", event.ID) + + return + } + + // Only text fields are translated. + if record.FieldType != models.FieldTypeText { + slog.Debug("translation: skip, not a text field", "feedback_record_id", record.ID) + + return + } + + // On create, only enqueue when there is text to translate. On update, enqueue even + // when value_text is now empty so the worker can clear a stale translation + // (mirrors the embedding provider). + if event.Type == datatypes.FeedbackRecordCreated && + (record.ValueText == nil || strings.TrimSpace(*record.ValueText) == "") { + slog.Debug("translation: skip, no value_text on create", "feedback_record_id", record.ID) + + return + } + + settings, err := p.resolver.GetSettings(ctx, record.TenantID) + if err != nil { + if p.metrics != nil { + p.metrics.RecordProviderError(ctx, "settings_read_failed") + } + + slog.Error("translation: resolve target language failed", + "event_id", event.ID, "feedback_record_id", record.ID, "error", err) + + return + } + + targetLang := settings.Settings.TargetLanguage + if targetLang == "" { + slog.Debug("translation: skip, tenant has no target language", "feedback_record_id", record.ID) + + return + } + + opts := &river.InsertOpts{ + Queue: p.queueName, + MaxAttempts: p.maxAttempts, + UniqueOpts: river.UniqueOpts{ByArgs: true, ByPeriod: uniqueByPeriodTranslation}, + } + + sourceLang := "" + if record.Language != nil { + sourceLang = *record.Language + } + + _, err = p.inserter.Insert(ctx, FeedbackTranslationArgs{ + FeedbackRecordID: record.ID, + TargetLang: targetLang, + ValueTextHash: translationContentHash(record.ValueText, sourceLang), + }, opts) + if err != nil { + if p.metrics != nil { + p.metrics.RecordProviderError(ctx, "enqueue_failed") + } + + slog.Error("translation: enqueue failed", + "event_id", event.ID, "feedback_record_id", record.ID, "error", err) + + return + } + + slog.Info("translation: job enqueued", + "event_id", event.ID, "feedback_record_id", record.ID, "target_lang", targetLang) + + if p.metrics != nil { + p.metrics.RecordJobsEnqueued(ctx, 1) + } +} + +// translationContentHash hashes the inputs that determine the translation — the +// trimmed, NFC-normalized value_text and the source language — for dedupe, so a +// source-language correction re-enqueues. Empty/nil value_text returns "empty" (a +// clear), independent of source language. +func translationContentHash(valueText *string, sourceLang string) string { + if valueText == nil { + return "empty" + } + + trimmed := strings.TrimSpace(*valueText) + if trimmed == "" { + return "empty" + } + + sum := sha256.Sum256([]byte(norm.NFC.String(trimmed) + "\x00" + strings.TrimSpace(sourceLang))) + + return hex.EncodeToString(sum[:]) +} diff --git a/internal/service/translation_provider_test.go b/internal/service/translation_provider_test.go new file mode 100644 index 00000000..5d313797 --- /dev/null +++ b/internal/service/translation_provider_test.go @@ -0,0 +1,298 @@ +package service + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/uuid" + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" + + "github.com/formbricks/hub/internal/datatypes" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/observability" +) + +// countingTranslationMetrics is an in-memory observability.TranslationMetrics for asserting +// which metrics fired (and with which reason/status labels). +type countingTranslationMetrics struct { + enqueued int64 + providerErr map[string]int + outcomes map[string]int + workerErr map[string]int + durations map[string]int +} + +func newCountingTranslationMetrics() *countingTranslationMetrics { + return &countingTranslationMetrics{ + providerErr: map[string]int{}, + outcomes: map[string]int{}, + workerErr: map[string]int{}, + durations: map[string]int{}, + } +} + +func (m *countingTranslationMetrics) RecordJobsEnqueued(_ context.Context, count int64) { + m.enqueued += count +} + +func (m *countingTranslationMetrics) RecordProviderError(_ context.Context, reason string) { + m.providerErr[reason]++ +} + +func (m *countingTranslationMetrics) RecordTranslationOutcome(_ context.Context, status string) { + m.outcomes[status]++ +} + +func (m *countingTranslationMetrics) RecordWorkerError(_ context.Context, reason string) { + m.workerErr[reason]++ +} + +func (m *countingTranslationMetrics) RecordTranslationDuration(_ context.Context, _ time.Duration, status string) { + m.durations[status]++ +} + +var _ observability.TranslationMetrics = (*countingTranslationMetrics)(nil) + +type mockTranslationInserter struct { + calls []FeedbackTranslationArgs + err error +} + +func (m *mockTranslationInserter) Insert( + _ context.Context, args river.JobArgs, _ *river.InsertOpts, +) (*rivertype.JobInsertResult, error) { + if a, ok := args.(FeedbackTranslationArgs); ok { + m.calls = append(m.calls, a) + } + + return &rivertype.JobInsertResult{}, m.err +} + +type mockTargetResolver struct { + target string + err error +} + +func (m *mockTargetResolver) GetSettings(_ context.Context, tenantID string) (*models.TenantSettings, error) { + if m.err != nil { + return nil, m.err + } + + return &models.TenantSettings{ + TenantID: tenantID, + Settings: models.EnrichmentSettings{TargetLanguage: m.target}, + }, nil +} + +func textRecord(valueText string) *models.FeedbackRecord { + return &models.FeedbackRecord{ + ID: uuid.New(), + TenantID: "org-1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + } +} + +func newTestTranslationProvider( + inserter RiverJobInserter, target string, resolveErr error, +) *TranslationProvider { + resolver := &mockTargetResolver{target: target, err: resolveErr} + + return NewTranslationProvider(inserter, resolver, TranslationsQueueName, 3, nil) +} + +func TestTranslationProvider_CreateWithTextEnqueues(t *testing.T) { + inserter := &mockTranslationInserter{} + provider := newTestTranslationProvider(inserter, "de-DE", nil) + record := textRecord("hello") + + provider.PublishEvent(context.Background(), Event{Type: datatypes.FeedbackRecordCreated, Data: record}) + + if len(inserter.calls) != 1 { + t.Fatalf("enqueued %d jobs, want 1", len(inserter.calls)) + } + + got := inserter.calls[0] + if got.FeedbackRecordID != record.ID || got.TargetLang != "de-DE" { + t.Fatalf("args = %+v, want record %s / target de-DE", got, record.ID) + } + + if got.ValueTextHash == "" || got.ValueTextHash == "empty" { + t.Fatalf("ValueTextHash = %q, want a content hash", got.ValueTextHash) + } +} + +func TestTranslationProvider_UpdateWithValueTextChangeEnqueues(t *testing.T) { + inserter := &mockTranslationInserter{} + provider := newTestTranslationProvider(inserter, "de-DE", nil) + + provider.PublishEvent(context.Background(), Event{ + Type: datatypes.FeedbackRecordUpdated, + Data: textRecord("hi"), + ChangedFields: []string{"value_text"}, + }) + + if len(inserter.calls) != 1 { + t.Fatalf("enqueued %d jobs, want 1", len(inserter.calls)) + } +} + +func TestTranslationProvider_UpdateToEmptyEnqueuesForClear(t *testing.T) { + inserter := &mockTranslationInserter{} + provider := newTestTranslationProvider(inserter, "de-DE", nil) + + // value_text cleared by an edit: enqueue so the worker can clear a stale translation. + provider.PublishEvent(context.Background(), Event{ + Type: datatypes.FeedbackRecordUpdated, + Data: textRecord(""), + ChangedFields: []string{"value_text"}, + }) + + if len(inserter.calls) != 1 { + t.Fatalf("enqueued %d jobs, want 1 (update-to-empty enqueues a clear)", len(inserter.calls)) + } + + if inserter.calls[0].ValueTextHash != "empty" { + t.Fatalf("ValueTextHash = %q, want \"empty\"", inserter.calls[0].ValueTextHash) + } +} + +func TestTranslationProvider_LanguageChangeEnqueues(t *testing.T) { + inserter := &mockTranslationInserter{} + provider := newTestTranslationProvider(inserter, "en-US", nil) + + // Correcting the source language (value_text unchanged) must re-enqueue: the + // translation depends on the source language. + provider.PublishEvent(context.Background(), Event{ + Type: datatypes.FeedbackRecordUpdated, + Data: textRecord("Bonjour"), + ChangedFields: []string{"language"}, + }) + + if len(inserter.calls) != 1 { + t.Fatalf("enqueued %d jobs, want 1 (a language change re-enqueues)", len(inserter.calls)) + } +} + +func TestTranslationContentHash_VariesBySourceLanguage(t *testing.T) { + text := "Bonjour" + + if translationContentHash(&text, "fr") == translationContentHash(&text, "en") { + t.Fatal("content hash must differ by source language so a language change re-translates") + } + + empty := "" + if got := translationContentHash(&empty, "fr"); got != "empty" { + t.Fatalf("blank value_text hash = %q, want \"empty\"", got) + } +} + +func TestTranslationProvider_RecordsMetrics(t *testing.T) { + t.Run("enqueue success increments jobs", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + inserter := &mockTranslationInserter{} + provider := NewTranslationProvider( + inserter, &mockTargetResolver{target: "de-DE"}, TranslationsQueueName, 3, metrics) + + provider.PublishEvent(context.Background(), Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord("hello")}) + + if metrics.enqueued != 1 { + t.Fatalf("enqueued = %d, want 1", metrics.enqueued) + } + }) + + t.Run("settings read failure records provider error", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + resolver := &mockTargetResolver{target: "de-DE", err: errors.New("boom")} + provider := NewTranslationProvider(&mockTranslationInserter{}, resolver, TranslationsQueueName, 3, metrics) + + provider.PublishEvent(context.Background(), Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord("hello")}) + + if metrics.providerErr["settings_read_failed"] != 1 { + t.Fatalf("settings_read_failed = %d, want 1", metrics.providerErr["settings_read_failed"]) + } + }) + + t.Run("enqueue failure records provider error and no enqueue", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + inserter := &mockTranslationInserter{err: errors.New("insert failed")} + provider := NewTranslationProvider( + inserter, &mockTargetResolver{target: "de-DE"}, TranslationsQueueName, 3, metrics) + + provider.PublishEvent(context.Background(), Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord("hello")}) + + if metrics.providerErr["enqueue_failed"] != 1 { + t.Fatalf("enqueue_failed = %d, want 1", metrics.providerErr["enqueue_failed"]) + } + + if metrics.enqueued != 0 { + t.Fatalf("enqueued = %d, want 0 on insert failure", metrics.enqueued) + } + }) +} + +func TestTranslationProvider_Skips(t *testing.T) { + tests := []struct { + name string + target string + resolveErr error + event Event + }{ + { + name: "empty value_text", + target: "de-DE", + event: Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord(" ")}, + }, + { + name: "non-text field", + target: "de-DE", + event: Event{Type: datatypes.FeedbackRecordCreated, Data: func() *models.FeedbackRecord { + r := textRecord("hello") + r.FieldType = models.FieldTypeCategorical + + return r + }()}, + }, + { + name: "tenant has no target language", + target: "", + event: Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord("hello")}, + }, + { + name: "resolver error", + target: "de-DE", + resolveErr: errors.New("boom"), + event: Event{Type: datatypes.FeedbackRecordCreated, Data: textRecord("hello")}, + }, + { + name: "update without value_text in changed fields", + target: "de-DE", + event: Event{ + Type: datatypes.FeedbackRecordUpdated, + Data: textRecord("hi"), + ChangedFields: []string{"metadata"}, + }, + }, + { + name: "event data is not a feedback record", + target: "de-DE", + event: Event{Type: datatypes.FeedbackRecordCreated, Data: "not a record"}, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + inserter := &mockTranslationInserter{} + provider := newTestTranslationProvider(inserter, testCase.target, testCase.resolveErr) + + provider.PublishEvent(context.Background(), testCase.event) + + if len(inserter.calls) != 0 { + t.Fatalf("enqueued %d jobs, want 0 (%s)", len(inserter.calls), testCase.name) + } + }) + } +} diff --git a/internal/workers/feedback_translation.go b/internal/workers/feedback_translation.go new file mode 100644 index 00000000..b80679cb --- /dev/null +++ b/internal/workers/feedback_translation.go @@ -0,0 +1,345 @@ +package workers + +import ( + "context" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/google/uuid" + "github.com/riverqueue/river" + "golang.org/x/text/language" + + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/observability" + "github.com/formbricks/hub/internal/service" +) + +// FeedbackTranslationWorker translates a feedback record's value_text into the tenant's +// target language and stores it, mirroring the embedding worker's error handling. +type FeedbackTranslationWorker struct { + river.WorkerDefaults[service.FeedbackTranslationArgs] + + service translationWorkerService + client service.TranslationClient + metrics observability.TranslationMetrics +} + +// translationWorkerService is the minimal interface the worker needs. +type translationWorkerService interface { + GetFeedbackRecord(ctx context.Context, id uuid.UUID) (*models.FeedbackRecord, error) + SetTranslation(ctx context.Context, feedbackRecordID uuid.UUID, translated *string, langKey string) error +} + +// NewFeedbackTranslationWorker creates a worker that fetches the record, translates its +// value_text, and stores the result. metrics may be nil when metrics are disabled. +func NewFeedbackTranslationWorker( + svc translationWorkerService, client service.TranslationClient, metrics observability.TranslationMetrics, +) *FeedbackTranslationWorker { + return &FeedbackTranslationWorker{service: svc, client: client, metrics: metrics} +} + +const feedbackTranslationTimeout = 30 * time.Second + +// Timeout limits how long a single translation job can run. +func (w *FeedbackTranslationWorker) Timeout(*river.Job[service.FeedbackTranslationArgs]) time.Duration { + return feedbackTranslationTimeout +} + +// Work loads the record, translates value_text into the target language (or copies it +// when the source already matches), and persists the result. +func (w *FeedbackTranslationWorker) Work(ctx context.Context, job *river.Job[service.FeedbackTranslationArgs]) error { + args := job.Args + start := time.Now() + + record, err := w.service.GetFeedbackRecord(ctx, args.FeedbackRecordID) + if err != nil { + // Not-found means the record was deleted or its tenant purged between enqueue and + // now: a benign race, not a failure. Record it as skipped (consistent with the + // not-found-on-write path) so it does not trip failure alerts. + if errors.Is(err, huberrors.ErrNotFound) { + if w.metrics != nil { + w.metrics.RecordTranslationOutcome(ctx, "skipped") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "skipped") + } + + slog.Info("translation: record gone before translate, skipping", + "feedback_record_id", args.FeedbackRecordID) + + return nil + } + + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "get_record_failed") + w.metrics.RecordTranslationOutcome(ctx, "failed_final") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "failed_final") + } + + slog.Error("translation: get record failed", "feedback_record_id", args.FeedbackRecordID, "error", err) + + return fmt.Errorf("get feedback record: %w", err) + } + + if record.FieldType != models.FieldTypeText { + if w.metrics != nil { + w.metrics.RecordTranslationOutcome(ctx, "skipped") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "skipped") + } + + slog.Info("translation: skipped, not a text field", "feedback_record_id", args.FeedbackRecordID) + + return nil + } + + // value_text became empty since enqueue (e.g. an edit cleared it): clear any stale + // translation rather than translate empty text (mirrors the embedding worker). + if record.ValueText == nil || strings.TrimSpace(*record.ValueText) == "" { + if err := w.service.SetTranslation(ctx, args.FeedbackRecordID, nil, ""); err != nil { + return w.handleSetTranslationError(ctx, err, args.FeedbackRecordID, start, job.Attempt >= job.MaxAttempts) + } + + if w.metrics != nil { + w.metrics.RecordTranslationOutcome(ctx, "success") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "success") + } + + slog.Info("translation: cleared (empty value_text)", "feedback_record_id", args.FeedbackRecordID) + + return nil + } + + translated, err := w.translate(ctx, record, args.TargetLang) + if err != nil { + return w.handleTranslateError(ctx, err, args.FeedbackRecordID, start, job) + } + + if err := w.service.SetTranslation(ctx, args.FeedbackRecordID, &translated, args.TargetLang); err != nil { + return w.handleSetTranslationError(ctx, err, args.FeedbackRecordID, start, job.Attempt >= job.MaxAttempts) + } + + if w.metrics != nil { + w.metrics.RecordTranslationOutcome(ctx, "success") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "success") + } + + slog.Info("translation: stored", + "feedback_record_id", args.FeedbackRecordID, "target_lang", args.TargetLang) + + return nil +} + +const ( + // defaultRateLimitSnooze applies when a provider 429 carried no retry-after hint. + defaultRateLimitSnooze = 30 * time.Second + // minRateLimitSnooze / maxRateLimitSnooze bound a single snooze so a tiny or absurd + // provider hint still yields a sane delay. + minRateLimitSnooze = 5 * time.Second + maxRateLimitSnooze = 5 * time.Minute + // maxRateLimitSnoozeWindow caps how long a job may keep snoozing against a standing rate + // limit before it is allowed to fail (and be recovered by a later backfill), so a + // permanently exhausted quota cannot snooze a single job forever. + maxRateLimitSnoozeWindow = time.Hour +) + +// rateLimitSnooze decides how long to snooze a rate-limited translation job. A provider 429 +// surfaces as a huberrors.RateLimitError; snoozing re-queues the job without consuming a retry +// attempt, so a burst against a rate-limited model defers rather than drops work. ok is false +// when err is not a rate-limit error, or when the job has been snoozing past +// maxRateLimitSnoozeWindow (then it should fail normally and let a backfill recover it). +func (w *FeedbackTranslationWorker) rateLimitSnooze( + err error, job *river.Job[service.FeedbackTranslationArgs], +) (time.Duration, bool) { + var rateLimited *huberrors.RateLimitError + if !errors.As(err, &rateLimited) { + return 0, false + } + + if !job.CreatedAt.IsZero() && time.Since(job.CreatedAt) > maxRateLimitSnoozeWindow { + return 0, false + } + + delay := rateLimited.RetryAfter + + switch { + case delay <= 0: + delay = defaultRateLimitSnooze + case delay < minRateLimitSnooze: + delay = minRateLimitSnooze + case delay > maxRateLimitSnooze: + delay = maxRateLimitSnooze + } + + return delay, true +} + +// handleTranslateError maps a translation provider failure to the right worker outcome: a +// rate-limit 429 snoozes (re-queues without consuming an attempt, so a burst defers rather than +// drops work); any other error retries until the attempts are spent, then fails. +func (w *FeedbackTranslationWorker) handleTranslateError( + ctx context.Context, err error, feedbackRecordID uuid.UUID, start time.Time, + job *river.Job[service.FeedbackTranslationArgs], +) error { + if delay, ok := w.rateLimitSnooze(err, job); ok { + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "rate_limited") + w.metrics.RecordTranslationOutcome(ctx, "retry") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "retry") + } + + slog.Warn("translation: provider rate-limited, snoozing", + "feedback_record_id", feedbackRecordID, "retry_after", delay.String()) + + //nolint:wrapcheck // river sentinel: JobSnooze must be returned unwrapped for River to detect the snooze + return river.JobSnooze(delay) + } + + isLastAttempt := job.Attempt >= job.MaxAttempts + outcome := "retry" + + if isLastAttempt { + outcome = "failed_final" + } + + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "translation_api_failed") + w.metrics.RecordTranslationOutcome(ctx, outcome) + w.metrics.RecordTranslationDuration(ctx, time.Since(start), outcome) + } + + if isLastAttempt { + slog.Error("translation: provider failed (final attempt)", + "feedback_record_id", feedbackRecordID, "error", err) + + return fmt.Errorf("translate (final attempt): %w", err) + } + + return fmt.Errorf("translate: %w", err) +} + +// translate returns the translated value_text, short-circuiting (copying the original) +// when the record's source language already matches the target language. +func (w *FeedbackTranslationWorker) translate( + ctx context.Context, record *models.FeedbackRecord, targetLang string, +) (string, error) { + sourceLang := "" + if record.Language != nil { + sourceLang = *record.Language + } + + if sameLanguageAndScript(sourceLang, targetLang) { + slog.Info("translation: source already in target language, copying value_text", + "feedback_record_id", record.ID) + + return *record.ValueText, nil + } + + translated, err := w.client.Translate(ctx, service.TranslateRequest{ + Text: *record.ValueText, + SourceLang: sourceLang, + TargetLang: targetLang, + }) + if err != nil { + return "", fmt.Errorf("translation client: %w", err) + } + + return translated, nil +} + +// handleSetTranslationError maps a translation write failure to a worker outcome, +// mirroring the embedding worker: a missing record completes the job (nothing to +// write), a tenant write conflict retries (the post-purge attempt finds the record +// gone), and anything else fails the job. +func (w *FeedbackTranslationWorker) handleSetTranslationError( + ctx context.Context, err error, feedbackRecordID uuid.UUID, start time.Time, isLastAttempt bool, +) error { + switch { + case errors.Is(err, huberrors.ErrTranslationSuperseded): + // The tenant's target language changed (or this job was enqueued from a stale + // settings cache) before the write landed: a newer-target job owns the row, so this + // stale-target write was a no-op. Benign — record as skipped, with a distinct worker + // label so target churn / cache staleness stays observable; do not retry or fail. + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "superseded") + w.metrics.RecordTranslationOutcome(ctx, "skipped") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "skipped") + } + + slog.Info("translation: superseded by newer target, skipping write", "feedback_record_id", feedbackRecordID) + + return nil + case errors.Is(err, huberrors.ErrNotFound): + if w.metrics != nil { + w.metrics.RecordTranslationOutcome(ctx, "skipped") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "skipped") + } + + slog.Info("translation: record gone before write, skipping", "feedback_record_id", feedbackRecordID) + + return nil + case errors.Is(err, huberrors.ErrTenantWriteConflict): + outcome := "retry" + if isLastAttempt { + outcome = "failed_final" + } + + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "tenant_write_conflict") + w.metrics.RecordTranslationOutcome(ctx, outcome) + w.metrics.RecordTranslationDuration(ctx, time.Since(start), outcome) + } + + slog.Warn("translation: tenant data purge in progress, deferring write", + "feedback_record_id", feedbackRecordID, "final_attempt", isLastAttempt) + + return fmt.Errorf("set feedback record translation: %w", err) + default: + if w.metrics != nil { + w.metrics.RecordWorkerError(ctx, "update_failed") + w.metrics.RecordTranslationOutcome(ctx, "failed_final") + w.metrics.RecordTranslationDuration(ctx, time.Since(start), "failed_final") + } + + slog.Error("translation: set translation failed", + "feedback_record_id", feedbackRecordID, "error", err) + + return fmt.Errorf("set feedback record translation: %w", err) + } +} + +// sameLanguageAndScript reports whether two BCP-47 tags share both base language and +// script, so en-US and en-GB match (copying the source is safe) but zh-Hans and +// zh-Hant do not (mutually unintelligible scripts must be translated). An empty or +// unparseable tag is treated as not matching, so an unknown source always translates. +func sameLanguageAndScript(source, target string) bool { + if strings.TrimSpace(source) == "" || strings.TrimSpace(target) == "" { + return false + } + + sourceTag, errSource := language.Parse(source) + targetTag, errTarget := language.Parse(target) + + if errSource != nil || errTarget != nil { + return false + } + + // "und" (and similar) coerce to a guessed base via likely-subtags; never treat an + // undetermined source or target as a match — translate instead of copying. + if sourceTag == language.Und || targetTag == language.Und { + return false + } + + sourceBase, _ := sourceTag.Base() + targetBase, _ := targetTag.Base() + + if sourceBase != targetBase { + return false + } + + sourceScript, _ := sourceTag.Script() + targetScript, _ := targetTag.Script() + + return sourceScript == targetScript +} diff --git a/internal/workers/feedback_translation_test.go b/internal/workers/feedback_translation_test.go new file mode 100644 index 00000000..69a5c6a1 --- /dev/null +++ b/internal/workers/feedback_translation_test.go @@ -0,0 +1,526 @@ +package workers + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/google/uuid" + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" + + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/observability" + "github.com/formbricks/hub/internal/service" +) + +// countingTranslationMetrics is an in-memory observability.TranslationMetrics for asserting +// which metrics fired (and with which reason/status labels). +type countingTranslationMetrics struct { + enqueued int64 + providerErr map[string]int + outcomes map[string]int + workerErr map[string]int + durations map[string]int +} + +func newCountingTranslationMetrics() *countingTranslationMetrics { + return &countingTranslationMetrics{ + providerErr: map[string]int{}, + outcomes: map[string]int{}, + workerErr: map[string]int{}, + durations: map[string]int{}, + } +} + +func (m *countingTranslationMetrics) RecordJobsEnqueued(_ context.Context, count int64) { + m.enqueued += count +} + +func (m *countingTranslationMetrics) RecordProviderError(_ context.Context, reason string) { + m.providerErr[reason]++ +} + +func (m *countingTranslationMetrics) RecordTranslationOutcome(_ context.Context, status string) { + m.outcomes[status]++ +} + +func (m *countingTranslationMetrics) RecordWorkerError(_ context.Context, reason string) { + m.workerErr[reason]++ +} + +func (m *countingTranslationMetrics) RecordTranslationDuration(_ context.Context, _ time.Duration, status string) { + m.durations[status]++ +} + +var _ observability.TranslationMetrics = (*countingTranslationMetrics)(nil) + +type translationSetCall struct { + translated *string + langKey string +} + +type mockTranslationWorkerService struct { + record *models.FeedbackRecord + getErr error + setErr error + setCalls []translationSetCall +} + +func (m *mockTranslationWorkerService) GetFeedbackRecord(_ context.Context, _ uuid.UUID) (*models.FeedbackRecord, error) { + return m.record, m.getErr +} + +func (m *mockTranslationWorkerService) SetTranslation( + _ context.Context, _ uuid.UUID, translated *string, langKey string, +) error { + m.setCalls = append(m.setCalls, translationSetCall{translated: translated, langKey: langKey}) + + return m.setErr +} + +type stubTranslationClient struct { + out string + err error + calls []service.TranslateRequest +} + +func (s *stubTranslationClient) Translate(_ context.Context, req service.TranslateRequest) (string, error) { + s.calls = append(s.calls, req) + + return s.out, s.err +} + +func translationRecord(valueText, sourceLang string) *models.FeedbackRecord { + record := &models.FeedbackRecord{ + ID: uuid.Must(uuid.NewV7()), + FieldType: models.FieldTypeText, + ValueText: &valueText, + } + if sourceLang != "" { + record.Language = &sourceLang + } + + return record +} + +func translationJob(targetLang string, attempt int) *river.Job[service.FeedbackTranslationArgs] { + return &river.Job[service.FeedbackTranslationArgs]{ + JobRow: &rivertype.JobRow{Attempt: attempt, MaxAttempts: 3}, + Args: service.FeedbackTranslationArgs{ + FeedbackRecordID: uuid.Must(uuid.NewV7()), + TargetLang: targetLang, + ValueTextHash: "hash", + }, + } +} + +func TestFeedbackTranslationWorker_TranslatesAndStores(t *testing.T) { + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour le monde", "fr")} + client := &stubTranslationClient{out: "Hello world"} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if len(client.calls) != 1 || client.calls[0].SourceLang != "fr" || client.calls[0].TargetLang != "en-US" { + t.Fatalf("client calls = %+v, want one fr->en-US", client.calls) + } + + if len(svc.setCalls) != 1 || svc.setCalls[0].translated == nil || + *svc.setCalls[0].translated != "Hello world" || svc.setCalls[0].langKey != "en-US" { + t.Fatalf("set calls = %+v, want translated 'Hello world' / en-US", svc.setCalls) + } +} + +func TestFeedbackTranslationWorker_SourceEqualsTargetCopies(t *testing.T) { + svc := &mockTranslationWorkerService{record: translationRecord("Hello", "en-US")} + client := &stubTranslationClient{out: "should-not-be-used"} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("en-GB", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if len(client.calls) != 0 { + t.Fatalf("client called %d times, want 0 (source base == target base)", len(client.calls)) + } + + if len(svc.setCalls) != 1 || svc.setCalls[0].translated == nil || *svc.setCalls[0].translated != "Hello" { + t.Fatalf("set calls = %+v, want copied 'Hello'", svc.setCalls) + } +} + +func TestFeedbackTranslationWorker_ClearsWhenValueTextEmpty(t *testing.T) { + svc := &mockTranslationWorkerService{record: translationRecord(" ", "fr")} + client := &stubTranslationClient{out: "x"} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if len(client.calls) != 0 { + t.Fatalf("client called %d times, want 0 (no translation of empty value_text)", len(client.calls)) + } + + if len(svc.setCalls) != 1 || svc.setCalls[0].translated != nil { + t.Fatalf("set calls = %+v, want one clear (nil translated)", svc.setCalls) + } +} + +func TestFeedbackTranslationWorker_UndeterminedSourceTranslates(t *testing.T) { + // "und" (undetermined) must not be treated as matching the target — translate, not copy. + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "und")} + client := &stubTranslationClient{out: "Hello"} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if len(client.calls) != 1 { + t.Fatalf("client called %d times, want 1 (und source must translate, not copy)", len(client.calls)) + } +} + +func TestFeedbackTranslationWorker_DifferentScriptTranslates(t *testing.T) { + // zh-Hans and zh-Hant share a base language but not a script: must translate, not copy. + svc := &mockTranslationWorkerService{record: translationRecord("simplified content", "zh-Hans")} + client := &stubTranslationClient{out: "translated"} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("zh-Hant", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if len(client.calls) != 1 { + t.Fatalf("client called %d times, want 1 (zh-Hans != zh-Hant must translate)", len(client.calls)) + } +} + +func TestFeedbackTranslationWorker_NotFoundCompletes(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{getErr: huberrors.NewNotFoundError("feedback record", "gone")} + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v, want nil (not-found completes)", err) + } + + // A not-found GET is a benign delete/purge race: record it as skipped, never + // failed_final (which would trip failure alerts) and not as a worker error. + if metrics.outcomes["skipped"] != 1 || metrics.outcomes["failed_final"] != 0 { + t.Fatalf("skipped=%d failed_final=%d, want 1/0", metrics.outcomes["skipped"], metrics.outcomes["failed_final"]) + } + + if metrics.workerErr["get_record_failed"] != 0 { + t.Fatalf("get_record_failed=%d, want 0 (not-found is not a worker error)", metrics.workerErr["get_record_failed"]) + } +} + +func TestFeedbackTranslationWorker_ProviderErrorRetriesThenFails(t *testing.T) { + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "fr")} + client := &stubTranslationClient{err: errors.New("api down")} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err == nil { + t.Fatal("Work() = nil on non-final attempt, want retry error") + } + + if err := worker.Work(context.Background(), translationJob("en-US", 3)); err == nil { + t.Fatal("Work() = nil on final attempt, want error") + } + + if len(svc.setCalls) != 0 { + t.Fatal("set called despite provider error") + } +} + +func TestFeedbackTranslationWorker_TenantWriteConflictRetries(t *testing.T) { + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour", "fr"), + setErr: huberrors.NewTenantWriteConflictError("purge in progress"), + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err == nil { + t.Fatal("Work() = nil, want retry on tenant write conflict") + } +} + +func TestFeedbackTranslationWorker_RecordGoneOnWriteCompletes(t *testing.T) { + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour", "fr"), + setErr: huberrors.NewNotFoundError("feedback record", "gone"), + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, nil) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v, want nil (record gone before write completes)", err) + } +} + +func TestFeedbackTranslationWorker_SupersededWriteSkips(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour le monde", "fr"), + setErr: huberrors.ErrTranslationSuperseded, + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hello world"}, metrics) + + // A stale-target write (the tenant's target changed before this job's write landed) is a + // benign no-op: the worker completes the job and records it as skipped, not failed. + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v, want nil (superseded write is a benign skip)", err) + } + + if metrics.outcomes["skipped"] != 1 || metrics.outcomes["failed_final"] != 0 { + t.Fatalf("skipped=%d failed_final=%d, want 1/0", + metrics.outcomes["skipped"], metrics.outcomes["failed_final"]) + } + + if metrics.workerErr["superseded"] != 1 { + t.Fatalf("workerErr[superseded] = %d, want 1", metrics.workerErr["superseded"]) + } +} + +func TestFeedbackTranslationWorker_RecordsMetrics(t *testing.T) { + t.Run("success records outcome and duration", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "fr")} + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if metrics.outcomes["success"] != 1 || metrics.durations["success"] != 1 { + t.Fatalf("success outcome=%d duration=%d, want 1/1", + metrics.outcomes["success"], metrics.durations["success"]) + } + }) + + t.Run("non-text field records skipped", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + record := translationRecord("hello", "fr") + record.FieldType = models.FieldTypeCategorical + worker := NewFeedbackTranslationWorker(&mockTranslationWorkerService{record: record}, &stubTranslationClient{}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if metrics.outcomes["skipped"] != 1 { + t.Fatalf("skipped = %d, want 1", metrics.outcomes["skipped"]) + } + }) + + t.Run("empty value_text clear records success", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{record: translationRecord(" ", "fr")} + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v", err) + } + + if metrics.outcomes["success"] != 1 { + t.Fatalf("clear success = %d, want 1", metrics.outcomes["success"]) + } + }) + + t.Run("provider failure records worker error, retry then failed_final", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "fr")} + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{err: errors.New("api down")}, metrics) + + _ = worker.Work(context.Background(), translationJob("en-US", 1)) // non-final -> retry + _ = worker.Work(context.Background(), translationJob("en-US", 3)) // final -> failed_final + + if metrics.workerErr["translation_api_failed"] != 2 { + t.Fatalf("translation_api_failed = %d, want 2", metrics.workerErr["translation_api_failed"]) + } + + if metrics.outcomes["retry"] != 1 || metrics.outcomes["failed_final"] != 1 { + t.Fatalf("retry=%d failed_final=%d, want 1/1", + metrics.outcomes["retry"], metrics.outcomes["failed_final"]) + } + }) + + t.Run("get record failure records get_record_failed and failed_final", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{getErr: errors.New("db down")} + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err == nil { + t.Fatal("Work() = nil, want error on get failure") + } + + if metrics.workerErr["get_record_failed"] != 1 || metrics.outcomes["failed_final"] != 1 { + t.Fatalf("get_record_failed=%d failed_final=%d, want 1/1", + metrics.workerErr["get_record_failed"], metrics.outcomes["failed_final"]) + } + }) + + t.Run("tenant write conflict records worker error and retry", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour", "fr"), + setErr: huberrors.NewTenantWriteConflictError("purge"), + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, metrics) + + _ = worker.Work(context.Background(), translationJob("en-US", 1)) + + if metrics.workerErr["tenant_write_conflict"] != 1 || metrics.outcomes["retry"] != 1 { + t.Fatalf("tenant_write_conflict=%d retry=%d, want 1/1", + metrics.workerErr["tenant_write_conflict"], metrics.outcomes["retry"]) + } + }) + + t.Run("set update failure records update_failed", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour", "fr"), + setErr: errors.New("update boom"), + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, metrics) + + _ = worker.Work(context.Background(), translationJob("en-US", 1)) + + if metrics.workerErr["update_failed"] != 1 || metrics.outcomes["failed_final"] != 1 { + t.Fatalf("update_failed=%d failed_final=%d, want 1/1", + metrics.workerErr["update_failed"], metrics.outcomes["failed_final"]) + } + }) + + t.Run("record gone on write records skipped", func(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{ + record: translationRecord("Bonjour", "fr"), + setErr: huberrors.NewNotFoundError("feedback record", "gone"), + } + worker := NewFeedbackTranslationWorker(svc, &stubTranslationClient{out: "Hi"}, metrics) + + if err := worker.Work(context.Background(), translationJob("en-US", 1)); err != nil { + t.Fatalf("Work() error = %v, want nil", err) + } + + if metrics.outcomes["skipped"] != 1 { + t.Fatalf("skipped = %d, want 1", metrics.outcomes["skipped"]) + } + }) +} + +func TestFeedbackTranslationWorker_RateLimitSnoozes(t *testing.T) { + metrics := newCountingTranslationMetrics() + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "fr")} + client := &stubTranslationClient{err: huberrors.NewRateLimitError(45*time.Second, errors.New("429"))} + worker := NewFeedbackTranslationWorker(svc, client, metrics) + + err := worker.Work(context.Background(), translationJob("en-US", 1)) + + var snooze *river.JobSnoozeError + if !errors.As(err, &snooze) { + t.Fatalf("Work() error = %v, want a river snooze error", err) + } + + if snooze.Duration != 45*time.Second { + t.Fatalf("snooze duration = %v, want 45s (provider retry-after)", snooze.Duration) + } + + if len(svc.setCalls) != 0 { + t.Fatalf("set called %d times on rate limit, want 0 (work deferred, not written)", len(svc.setCalls)) + } + + // A snooze defers, it does not fail: record the rate_limited worker error and a retry + // outcome, never failed_final (which would trip failure alerts). + if metrics.workerErr["rate_limited"] != 1 || metrics.outcomes["retry"] != 1 || metrics.outcomes["failed_final"] != 0 { + t.Fatalf("rate_limited=%d retry=%d failed_final=%d, want 1/1/0", + metrics.workerErr["rate_limited"], metrics.outcomes["retry"], metrics.outcomes["failed_final"]) + } +} + +func TestFeedbackTranslationWorker_RateLimitSnoozesOnLastAttempt(t *testing.T) { + // A rate limit must snooze even when River's retry attempts are exhausted: snoozing does + // not consume an attempt, so a 429 defers work instead of being dropped as failed_final. + svc := &mockTranslationWorkerService{record: translationRecord("Bonjour", "fr")} + client := &stubTranslationClient{err: huberrors.NewRateLimitError(0, errors.New("429"))} + worker := NewFeedbackTranslationWorker(svc, client, nil) + + err := worker.Work(context.Background(), translationJob("en-US", 3)) + + var snooze *river.JobSnoozeError + if !errors.As(err, &snooze) { + t.Fatalf("Work() error = %v, want a river snooze error on the last attempt", err) + } + + if snooze.Duration != defaultRateLimitSnooze { + t.Fatalf("snooze duration = %v, want the default %v (no provider hint)", snooze.Duration, defaultRateLimitSnooze) + } +} + +func TestRateLimitSnooze(t *testing.T) { + worker := NewFeedbackTranslationWorker(&mockTranslationWorkerService{}, &stubTranslationClient{}, nil) + + tests := []struct { + name string + err error + createdAt time.Time + wantDelay time.Duration + wantOK bool + }{ + {name: "not a rate-limit error", err: errors.New("boom"), wantOK: false}, + { + name: "provider hint honored", + err: huberrors.NewRateLimitError(45*time.Second, nil), + wantDelay: 45 * time.Second, + wantOK: true, + }, + { + name: "no hint uses default", + err: huberrors.NewRateLimitError(0, nil), + wantDelay: defaultRateLimitSnooze, + wantOK: true, + }, + { + name: "tiny hint clamped up to min", + err: huberrors.NewRateLimitError(time.Second, nil), + wantDelay: minRateLimitSnooze, + wantOK: true, + }, + { + name: "huge hint clamped down to max", + err: huberrors.NewRateLimitError(time.Hour, nil), + wantDelay: maxRateLimitSnooze, + wantOK: true, + }, + { + name: "past the snooze window fails normally", + err: huberrors.NewRateLimitError(30*time.Second, nil), + createdAt: time.Now().Add(-2 * maxRateLimitSnoozeWindow), + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + job := &river.Job[service.FeedbackTranslationArgs]{ + JobRow: &rivertype.JobRow{Attempt: 1, MaxAttempts: 3, CreatedAt: tt.createdAt}, + } + + delay, ok := worker.rateLimitSnooze(tt.err, job) + if ok != tt.wantOK { + t.Fatalf("rateLimitSnooze ok = %v, want %v", ok, tt.wantOK) + } + + if tt.wantOK && delay != tt.wantDelay { + t.Fatalf("rateLimitSnooze delay = %v, want %v", delay, tt.wantDelay) + } + }) + } +} diff --git a/internal/workers/tenant_translation_backfill.go b/internal/workers/tenant_translation_backfill.go new file mode 100644 index 00000000..6d18acc7 --- /dev/null +++ b/internal/workers/tenant_translation_backfill.go @@ -0,0 +1,69 @@ +package workers + +import ( + "context" + "fmt" + "log/slog" + "time" + + "github.com/jackc/pgx/v5" + "github.com/riverqueue/river" + + "github.com/formbricks/hub/internal/service" +) + +// tenantTranslationBackfillService is the minimal interface the worker needs. +type tenantTranslationBackfillService interface { + BackfillTranslationsForTenant( + ctx context.Context, inserter service.RiverJobInserter, queueName string, maxAttempts int, tenantID string, + ) (int, error) +} + +// TenantTranslationBackfillWorker fans out a per-tenant re-translation: it lists the +// tenant's stale text records and enqueues a FeedbackTranslationArgs job for each. It is +// enqueued when a tenant's translation settings change (see +// service.TenantTranslationBackfillArgs) and runs off the request path. +type TenantTranslationBackfillWorker struct { + river.WorkerDefaults[service.TenantTranslationBackfillArgs] + + service tenantTranslationBackfillService + maxAttempts int +} + +// NewTenantTranslationBackfillWorker creates the worker. maxAttempts is applied to the +// per-record translation jobs it enqueues. +func NewTenantTranslationBackfillWorker( + svc tenantTranslationBackfillService, maxAttempts int, +) *TenantTranslationBackfillWorker { + return &TenantTranslationBackfillWorker{service: svc, maxAttempts: maxAttempts} +} + +// tenantTranslationBackfillTimeout bounds a single tenant fan-out. River rescues a job that +// exceeds it; on retry the idempotent backfill query re-lists only the not-yet-enqueued +// tail, so a killed fan-out self-heals without continuation bookkeeping. +const tenantTranslationBackfillTimeout = 5 * time.Minute + +// Timeout limits how long a single tenant backfill fan-out can run. +func (w *TenantTranslationBackfillWorker) Timeout(*river.Job[service.TenantTranslationBackfillArgs]) time.Duration { + return tenantTranslationBackfillTimeout +} + +// Work lists the tenant's stale records and enqueues per-record translation jobs onto the +// translations queue. The River client is obtained from the context (the only place River +// sets it) and handed to the service as the inserter, preserving the injected-inserter seam. +func (w *TenantTranslationBackfillWorker) Work( + ctx context.Context, job *river.Job[service.TenantTranslationBackfillArgs], +) error { + client := river.ClientFromContext[pgx.Tx](ctx) + + enqueued, err := w.service.BackfillTranslationsForTenant( + ctx, client, service.TranslationsQueueName, w.maxAttempts, job.Args.TenantID) + if err != nil { + return fmt.Errorf("backfill translations for tenant %s: %w", job.Args.TenantID, err) + } + + slog.Info("translation backfill: tenant fan-out complete", + "tenant_id", job.Args.TenantID, "enqueued", enqueued) + + return nil +} diff --git a/internal/workers/wiring.go b/internal/workers/wiring.go index 19bda623..0458d6c4 100644 --- a/internal/workers/wiring.go +++ b/internal/workers/wiring.go @@ -24,6 +24,14 @@ type RiverDeps struct { EmbeddingClient service.EmbeddingClient EmbeddingDocPrefix string EmbeddingMetrics observability.EmbeddingMetrics + + // Translation worker (optional; if TranslationClient is nil, translation worker is not registered) + TranslationService translationWorkerService + TranslationClient service.TranslationClient + TranslationMetrics observability.TranslationMetrics + // Per-tenant translation backfill worker (registered alongside the translation worker). + TranslationBackfillService tenantTranslationBackfillService + TranslationMaxAttempts int } // NewRiverWorkersAndQueues builds River workers and queue config from cfg and deps. @@ -39,10 +47,12 @@ func NewRiverWorkersAndQueues( maxDefault := cfg.Webhook.DeliveryMaxConcurrent maxEmbedding := cfg.Embedding.MaxConcurrent + maxTranslation := cfg.Translation.MaxConcurrent if placeholderMaxWorkers > 0 { maxDefault = placeholderMaxWorkers maxEmbedding = placeholderMaxWorkers + maxTranslation = placeholderMaxWorkers } queues := map[string]river.QueueConfig{ @@ -56,5 +66,17 @@ func NewRiverWorkersAndQueues( queues[service.EmbeddingsQueueName] = river.QueueConfig{MaxWorkers: maxEmbedding} } + if deps.TranslationClient != nil { + translationWorker := NewFeedbackTranslationWorker(deps.TranslationService, deps.TranslationClient, deps.TranslationMetrics) + river.AddWorker(workers, translationWorker) + + queues[service.TranslationsQueueName] = river.QueueConfig{MaxWorkers: maxTranslation} + + backfillWorker := NewTenantTranslationBackfillWorker(deps.TranslationBackfillService, deps.TranslationMaxAttempts) + river.AddWorker(workers, backfillWorker) + + queues[service.TranslationBackfillsQueueName] = river.QueueConfig{MaxWorkers: maxTranslation} + } + return workers, queues } diff --git a/migrations/013_add_feedback_record_translation.sql b/migrations/013_add_feedback_record_translation.sql new file mode 100644 index 00000000..91a1194f --- /dev/null +++ b/migrations/013_add_feedback_record_translation.sql @@ -0,0 +1,12 @@ +-- +goose up +-- Language enrichment (ENG-1255): translated copy of an open-text answer plus the +-- target locale it was produced in. Both are server-generated by the async +-- translation worker; they are NULL until a tenant configures a target language and +-- the record is enriched. The original value_text is preserved (embeddings continue +-- to operate on the source-language text). +ALTER TABLE feedback_records ADD COLUMN value_text_translated TEXT; +ALTER TABLE feedback_records ADD COLUMN translation_lang_key VARCHAR(35); + +-- +goose down +ALTER TABLE feedback_records DROP COLUMN IF EXISTS translation_lang_key; +ALTER TABLE feedback_records DROP COLUMN IF EXISTS value_text_translated; diff --git a/openapi.yaml b/openapi.yaml index e8e77888..11b21ed5 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1868,6 +1868,10 @@ components: type: string description: Tenant/organization identifier. NULL bytes not allowed. pattern: '^[^\x00]*$' + translation_lang_key: + type: string + description: BCP-47 target locale that value_text_translated was produced in (language enrichment). Read-only; absent until the record is enriched. + maxLength: 35 updated_at: type: string description: When this record was last updated @@ -1890,6 +1894,10 @@ components: type: string description: Text response. NULL bytes not allowed. pattern: '^[^\x00]*$' + value_text_translated: + type: string + description: value_text translated into the tenant's configured target language (language enrichment). Read-only; absent until the record is enriched. + pattern: '^[^\x00]*$' required: - id - collected_at diff --git a/tests/feedback_translation_test.go b/tests/feedback_translation_test.go new file mode 100644 index 00000000..0c6dd612 --- /dev/null +++ b/tests/feedback_translation_test.go @@ -0,0 +1,364 @@ +package tests + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/riverqueue/river" + "github.com/riverqueue/river/rivertype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/formbricks/hub/internal/config" + "github.com/formbricks/hub/internal/huberrors" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/repository" + "github.com/formbricks/hub/internal/service" + "github.com/formbricks/hub/internal/workers" + "github.com/formbricks/hub/pkg/database" +) + +// TestFeedbackRecords_SetTranslation locks the SetTranslation write contract: it +// persists the translated text + target locale (round-tripping through GetByID and +// the shared scanFeedbackRecord), clears them when the translation is nil, leaves +// the source value_text untouched, and returns NotFound for a missing record. The +// async worker exercises this end-to-end; this covers the repo paths directly, +// including the not-found path the worker test would not reach. +func TestFeedbackRecords_SetTranslation(t *testing.T) { + ctx := context.Background() + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, database.WithPoolConfig(cfg.Database.PoolConfig())) + require.NoError(t, err) + + defer db.Close() + + repo := repository.NewFeedbackRecordsRepository(db) + + tenantID := testTenantID("set-translation") + valueText := "Hello, world" + + // A translation write lands only while it matches the tenant's current target_language, + // so seed that target before exercising the write contract. + settingsRepo := repository.NewTenantSettingsRepository(db) + _, err = settingsRepo.Upsert(ctx, tenantID, models.EnrichmentSettings{TargetLanguage: "de-DE"}) + require.NoError(t, err) + + created, err := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("submission"), + }) + require.NoError(t, err) + + // A fresh record has no translation yet. + require.Nil(t, created.ValueTextTranslated) + require.Nil(t, created.TranslationLangKey) + + // Success: translated text + target locale persist and round-trip via GetByID. + translated := "Hallo, Welt" + require.NoError(t, repo.SetTranslation(ctx, created.ID, &translated, "de-DE")) + + got, err := repo.GetByID(ctx, created.ID) + require.NoError(t, err) + require.NotNil(t, got.ValueTextTranslated) + assert.Equal(t, "Hallo, Welt", *got.ValueTextTranslated) + require.NotNil(t, got.TranslationLangKey) + assert.Equal(t, "de-DE", *got.TranslationLangKey) + require.NotNil(t, got.ValueText) + assert.Equal(t, "Hello, world", *got.ValueText, "source value_text must be preserved") + + // Clearing: a nil translation nulls the column. + require.NoError(t, repo.SetTranslation(ctx, created.ID, nil, "de-DE")) + + cleared, err := repo.GetByID(ctx, created.ID) + require.NoError(t, err) + assert.Nil(t, cleared.ValueTextTranslated, "nil translation clears value_text_translated") + assert.Nil(t, cleared.TranslationLangKey, "clearing also nulls translation_lang_key") + + // Missing record: NotFound (resolved via the shared tenant write lock). + err = repo.SetTranslation(ctx, uuid.New(), &translated, "de-DE") + require.Error(t, err) + assert.ErrorIs(t, err, huberrors.ErrNotFound, "a missing record returns NotFound") +} + +// TestFeedbackRecords_SetTranslation_StaleTargetIsNoOp locks the out-of-order-write guard: a +// translation whose target no longer matches the tenant's current target_language (an older +// job finishing after a target change / stale-cache enqueue) is a no-op that returns +// ErrTranslationSuperseded and never clobbers the current translation. +func TestFeedbackRecords_SetTranslation_StaleTargetIsNoOp(t *testing.T) { + ctx := context.Background() + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, database.WithPoolConfig(cfg.Database.PoolConfig())) + require.NoError(t, err) + + defer db.Close() + + repo := repository.NewFeedbackRecordsRepository(db) + settingsRepo := repository.NewTenantSettingsRepository(db) + + tenantID := testTenantID("stale-translation") + valueText := "Hello, world" + + // Tenant's current target is de-DE. + _, err = settingsRepo.Upsert(ctx, tenantID, models.EnrichmentSettings{TargetLanguage: "de-DE"}) + require.NoError(t, err) + + created, err := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("submission"), + }) + require.NoError(t, err) + + // A stale-target write (fr-FR — an older job or a stale-cache enqueue) must not land. + stale := "Bonjour le monde" + err = repo.SetTranslation(ctx, created.ID, &stale, "fr-FR") + require.ErrorIs(t, err, huberrors.ErrTranslationSuperseded, "stale-target write is superseded") + + got, err := repo.GetByID(ctx, created.ID) + require.NoError(t, err) + assert.Nil(t, got.ValueTextTranslated, "stale-target write must not set a translation") + assert.Nil(t, got.TranslationLangKey, "stale-target write must not set a lang key") + + // A current-target write (de-DE) lands normally. + current := "Hallo, Welt" + require.NoError(t, repo.SetTranslation(ctx, created.ID, ¤t, "de-DE")) + + got, err = repo.GetByID(ctx, created.ID) + require.NoError(t, err) + require.NotNil(t, got.TranslationLangKey) + assert.Equal(t, "de-DE", *got.TranslationLangKey) + require.NotNil(t, got.ValueTextTranslated) + assert.Equal(t, "Hallo, Welt", *got.ValueTextTranslated) + + // After the tenant switches its target to fr-FR, the previously-stale fr-FR write now + // lands — proving the guard tracks the current target rather than a fixed value. + _, err = settingsRepo.Upsert(ctx, tenantID, models.EnrichmentSettings{TargetLanguage: "fr-FR"}) + require.NoError(t, err) + + require.NoError(t, repo.SetTranslation(ctx, created.ID, &stale, "fr-FR")) + + got, err = repo.GetByID(ctx, created.ID) + require.NoError(t, err) + require.NotNil(t, got.TranslationLangKey) + assert.Equal(t, "fr-FR", *got.TranslationLangKey) + require.NotNil(t, got.ValueTextTranslated) + assert.Equal(t, "Bonjour le monde", *got.ValueTextTranslated) +} + +// TestFeedbackRecords_ListTranslationBackfillTargets verifies the backfill query: it +// returns text records whose tenant has a target language and whose stored +// translation_lang_key differs from it (never translated, or stale), and excludes +// records already current and tenants with no target. +func TestFeedbackRecords_ListTranslationBackfillTargets(t *testing.T) { + ctx := context.Background() + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, database.WithPoolConfig(cfg.Database.PoolConfig())) + require.NoError(t, err) + + defer db.Close() + + repo := repository.NewFeedbackRecordsRepository(db) + settingsRepo := repository.NewTenantSettingsRepository(db) + + tenantWithTarget := testTenantID("backfill-target") + tenantNoTarget := testTenantID("backfill-notarget") + + _, err = settingsRepo.Upsert(ctx, tenantWithTarget, models.EnrichmentSettings{TargetLanguage: "de-DE"}) + require.NoError(t, err) + + makeTextRecord := func(tenantID, valueText string) *models.FeedbackRecord { + rec, createErr := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("sub"), + }) + require.NoError(t, createErr) + + return rec + } + + untranslated := makeTextRecord(tenantWithTarget, "needs translation") + stale := makeTextRecord(tenantWithTarget, "stale translation") + current := makeTextRecord(tenantWithTarget, "already current") + noTarget := makeTextRecord(tenantNoTarget, "tenant has no target") + + // Seed a stale translation: SetTranslation only persists a write matching the tenant's + // current target, so briefly switch the target to fr-FR, write, then restore de-DE — the + // stored fr-FR is now stale relative to the de-DE target the backfill query reads. + altTranslation := "alt" + _, err = settingsRepo.Upsert(ctx, tenantWithTarget, models.EnrichmentSettings{TargetLanguage: "fr-FR"}) + require.NoError(t, err) + require.NoError(t, repo.SetTranslation(ctx, stale.ID, &altTranslation, "fr-FR")) + _, err = settingsRepo.Upsert(ctx, tenantWithTarget, models.EnrichmentSettings{TargetLanguage: "de-DE"}) + require.NoError(t, err) + + currentTranslation := "Hallo" + require.NoError(t, repo.SetTranslation(ctx, current.ID, ¤tTranslation, "de-DE")) // matches target + + targets, err := repo.ListTranslationBackfillTargets(ctx, uuid.Nil, 100) + require.NoError(t, err) + + byID := make(map[uuid.UUID]string, len(targets)) + for _, target := range targets { + byID[target.FeedbackRecordID] = target.TargetLang + } + + assert.Equal(t, "de-DE", byID[untranslated.ID], "untranslated record is a backfill target for de-DE") + assert.Equal(t, "de-DE", byID[stale.ID], "stale translation (fr-FR != de-DE) is a backfill target") + + _, currentPresent := byID[current.ID] + assert.False(t, currentPresent, "record already translated to the current target must be excluded") + + _, noTargetPresent := byID[noTarget.ID] + assert.False(t, noTargetPresent, "record whose tenant has no target must be excluded") +} + +type fakeTranslationClient struct { + out string + calls int +} + +func (f *fakeTranslationClient) Translate(_ context.Context, _ service.TranslateRequest) (string, error) { + f.calls++ + + return f.out, nil +} + +func translationWorkerJob(recordID uuid.UUID, targetLang string) *river.Job[service.FeedbackTranslationArgs] { + return &river.Job[service.FeedbackTranslationArgs]{ + JobRow: &rivertype.JobRow{Attempt: 1, MaxAttempts: 3}, + Args: service.FeedbackTranslationArgs{ + FeedbackRecordID: recordID, + TargetLang: targetLang, + ValueTextHash: "h", + }, + } +} + +// TestFeedbackTranslation_WorkerPipeline drives FeedbackTranslationWorker end to end +// against Postgres (with a fake translation client): it translates and persists, +// copies verbatim when the source already matches the target (no provider call), and +// clears a stale translation when value_text is empty. +func TestFeedbackTranslation_WorkerPipeline(t *testing.T) { + ctx := context.Background() + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool(ctx, cfg.Database.URL, database.WithPoolConfig(cfg.Database.PoolConfig())) + require.NoError(t, err) + + defer db.Close() + + repo := repository.NewFeedbackRecordsRepository(db) + settingsRepo := repository.NewTenantSettingsRepository(db) + svc := service.NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + // Each record gets a fresh tenant; seed that tenant's target so the translation write + // (which now only lands while it matches the current target) succeeds. + createText := func(valueText, sourceLang, target string) *models.FeedbackRecord { + req := &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: testTenantID("worker-pipeline"), + SubmissionID: testTenantID("sub"), + } + if sourceLang != "" { + req.Language = &sourceLang + } + + rec, createErr := repo.Create(ctx, req) + require.NoError(t, createErr) + + if target != "" { + _, seedErr := settingsRepo.Upsert(ctx, rec.TenantID, models.EnrichmentSettings{TargetLanguage: target}) + require.NoError(t, seedErr) + } + + return rec + } + + t.Run("translates and persists", func(t *testing.T) { + rec := createText("Bonjour le monde", "fr", "en-US") + fake := &fakeTranslationClient{out: "Hello world"} + worker := workers.NewFeedbackTranslationWorker(svc, fake, nil) + + require.NoError(t, worker.Work(ctx, translationWorkerJob(rec.ID, "en-US"))) + + got, getErr := repo.GetByID(ctx, rec.ID) + require.NoError(t, getErr) + require.NotNil(t, got.ValueTextTranslated) + assert.Equal(t, "Hello world", *got.ValueTextTranslated) + require.NotNil(t, got.TranslationLangKey) + assert.Equal(t, "en-US", *got.TranslationLangKey) + assert.Equal(t, 1, fake.calls) + require.NotNil(t, got.ValueText) + assert.Equal(t, "Bonjour le monde", *got.ValueText, "source value_text preserved") + }) + + t.Run("copies when source matches target without calling the provider", func(t *testing.T) { + rec := createText("Hello world", "en-US", "en-GB") + fake := &fakeTranslationClient{out: "should-not-be-used"} + worker := workers.NewFeedbackTranslationWorker(svc, fake, nil) + + require.NoError(t, worker.Work(ctx, translationWorkerJob(rec.ID, "en-GB"))) + + got, getErr := repo.GetByID(ctx, rec.ID) + require.NoError(t, getErr) + require.NotNil(t, got.ValueTextTranslated) + assert.Equal(t, "Hello world", *got.ValueTextTranslated, "source copied verbatim") + assert.Equal(t, 0, fake.calls, "no provider call when source base+script == target") + }) + + t.Run("clears a stale translation when value_text is empty", func(t *testing.T) { + rec := createText("", "fr", "en-US") + stale := "stale translation" + require.NoError(t, repo.SetTranslation(ctx, rec.ID, &stale, "en-US")) + + worker := workers.NewFeedbackTranslationWorker(svc, &fakeTranslationClient{out: "unused"}, nil) + require.NoError(t, worker.Work(ctx, translationWorkerJob(rec.ID, "en-US"))) + + got, getErr := repo.GetByID(ctx, rec.ID) + require.NoError(t, getErr) + assert.Nil(t, got.ValueTextTranslated, "stale translation cleared when value_text is empty") + assert.Nil(t, got.TranslationLangKey, "clearing path must also null translation_lang_key") + }) + + t.Run("stale-target write is superseded without persisting", func(t *testing.T) { + rec := createText("Hallo Welt", "de", "de-DE") // tenant's current target is de-DE + + worker := workers.NewFeedbackTranslationWorker(svc, &fakeTranslationClient{out: "Bonjour le monde"}, nil) + + // The job's target (fr-FR) no longer matches the tenant's current target (de-DE) — an + // older job or a stale-cache enqueue. The write must no-op and the job must complete + // (not fail or retry), leaving the record untranslated. + require.NoError(t, worker.Work(ctx, translationWorkerJob(rec.ID, "fr-FR"))) + + got, getErr := repo.GetByID(ctx, rec.ID) + require.NoError(t, getErr) + assert.Nil(t, got.ValueTextTranslated, "stale-target translation must not be persisted") + assert.Nil(t, got.TranslationLangKey, "stale-target lang key must not be persisted") + }) +} diff --git a/tests/tenant_translation_backfill_test.go b/tests/tenant_translation_backfill_test.go new file mode 100644 index 00000000..15d47dcf --- /dev/null +++ b/tests/tenant_translation_backfill_test.go @@ -0,0 +1,229 @@ +package tests + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + "github.com/riverqueue/river" + "github.com/riverqueue/river/riverdriver/riverpgxv5" + "github.com/riverqueue/river/rivertest" + "github.com/riverqueue/river/rivertype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/formbricks/hub/internal/config" + "github.com/formbricks/hub/internal/models" + "github.com/formbricks/hub/internal/repository" + "github.com/formbricks/hub/internal/service" + "github.com/formbricks/hub/internal/workers" + "github.com/formbricks/hub/pkg/database" +) + +// countingTranslationInserter records the FeedbackTranslationArgs jobs enqueued. +type countingTranslationInserter struct { + args []service.FeedbackTranslationArgs +} + +func (c *countingTranslationInserter) Insert( + _ context.Context, args river.JobArgs, _ *river.InsertOpts, +) (*rivertype.JobInsertResult, error) { + if a, ok := args.(service.FeedbackTranslationArgs); ok { + c.args = append(c.args, a) + } + + return &rivertype.JobInsertResult{}, nil +} + +// backfillTestEnv opens the test DB and seeds two tenants, each with a target language. +func backfillTestEnv(t *testing.T) (*pgxpool.Pool, *repository.FeedbackRecordsRepository, string, string) { + t.Helper() + + cfg, err := config.Load() + require.NoError(t, err) + + db, err := database.NewPostgresPool( + context.Background(), cfg.Database.URL, database.WithPoolConfig(cfg.Database.PoolConfig())) + require.NoError(t, err) + + t.Cleanup(db.Close) + + repo := repository.NewFeedbackRecordsRepository(db) + settingsRepo := repository.NewTenantSettingsRepository(db) + + tenantA := testTenantID("bf-a") + tenantB := testTenantID("bf-b") + + _, err = settingsRepo.Upsert(context.Background(), tenantA, models.EnrichmentSettings{TargetLanguage: "de-DE"}) + require.NoError(t, err) + + _, err = settingsRepo.Upsert(context.Background(), tenantB, models.EnrichmentSettings{TargetLanguage: "fr-FR"}) + require.NoError(t, err) + + return db, repo, tenantA, tenantB +} + +// TestListTranslationBackfillTargetsForTenant locks the new keyset query: it returns only +// the named tenant's stale text records (tenant isolation — the global query had none), +// excludes records already at the current target (idempotency), and tiles a >limit set +// across keyset pages with no gaps or overlaps. +func TestListTranslationBackfillTargetsForTenant(t *testing.T) { + ctx := context.Background() + _, repo, tenantA, tenantB := backfillTestEnv(t) + + makeText := func(tenantID, valueText string) *models.FeedbackRecord { + rec, err := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("sub"), + }) + require.NoError(t, err) + + return rec + } + + // Tenant A: three untranslated text records (created in order → ascending UUIDv7 ids). + recA1 := makeText(tenantA, "one") + recA2 := makeText(tenantA, "two") + recA3 := makeText(tenantA, "three") + + // Tenant A: one already translated to the current target (de-DE) → must be excluded. + recCurrent := makeText(tenantA, "current") + currentTranslation := "Aktuell" + require.NoError(t, repo.SetTranslation(ctx, recCurrent.ID, ¤tTranslation, "de-DE")) + + // Tenant B: one untranslated record → must never appear for tenant A. + recB := makeText(tenantB, "bee") + + page1, err := repo.ListTranslationBackfillTargetsForTenant(ctx, tenantA, uuid.Nil, 2) + require.NoError(t, err) + require.Len(t, page1, 2, "first keyset page should be full") + + page2, err := repo.ListTranslationBackfillTargetsForTenant(ctx, tenantA, page1[1].FeedbackRecordID, 2) + require.NoError(t, err) + require.Len(t, page2, 1, "second page holds the remaining stale record") + + page3, err := repo.ListTranslationBackfillTargetsForTenant(ctx, tenantA, page2[0].FeedbackRecordID, 2) + require.NoError(t, err) + assert.Empty(t, page3, "no more pages") + + got := map[uuid.UUID]string{} + for _, target := range page1 { + got[target.FeedbackRecordID] = target.TargetLang + } + + for _, target := range page2 { + got[target.FeedbackRecordID] = target.TargetLang + } + + assert.Equal(t, map[uuid.UUID]string{ + recA1.ID: "de-DE", recA2.ID: "de-DE", recA3.ID: "de-DE", + }, got, "exactly tenant A's three stale records, all targeting de-DE") + + assert.NotContains(t, got, recCurrent.ID, "record already at the current target is excluded (idempotency)") + assert.NotContains(t, got, recB.ID, "another tenant's record never appears (tenant isolation)") +} + +// TestBackfillTranslationsForTenant_Isolation drives the service against the real DB with a +// recording inserter: a per-tenant backfill enqueues a translation job for exactly that +// tenant's stale records and nothing for any other tenant. +func TestBackfillTranslationsForTenant_Isolation(t *testing.T) { + ctx := context.Background() + _, repo, tenantA, tenantB := backfillTestEnv(t) + + makeText := func(tenantID, valueText string) *models.FeedbackRecord { + rec, err := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("sub"), + }) + require.NoError(t, err) + + return rec + } + + recA1 := makeText(tenantA, "one") + recA2 := makeText(tenantA, "two") + makeText(tenantB, "bee") // other tenant, must be untouched + + inserter := &countingTranslationInserter{} + svc := service.NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + + enqueued, err := svc.BackfillTranslationsForTenant(ctx, inserter, service.TranslationsQueueName, 3, tenantA) + require.NoError(t, err) + assert.Equal(t, 2, enqueued) + + enqueuedIDs := map[uuid.UUID]string{} + for _, job := range inserter.args { + enqueuedIDs[job.FeedbackRecordID] = job.TargetLang + + assert.Equal(t, "backfill", job.ValueTextHash, "backfill jobs carry the constant hash") + } + + assert.Equal(t, map[uuid.UUID]string{recA1.ID: "de-DE", recA2.ID: "de-DE"}, enqueuedIDs, + "only tenant A's stale records are enqueued, targeting de-DE") +} + +// TestTenantTranslationBackfillWorker_Work drives the worker end to end: given a +// TenantTranslationBackfillArgs and a River client on the context (the seam the worker uses +// to enqueue), it inserts a feedback_translation job for each of the tenant's stale records. +func TestTenantTranslationBackfillWorker_Work(t *testing.T) { + ctx := context.Background() + pool, repo, tenantA, _ := backfillTestEnv(t) + + makeText := func(tenantID, valueText string) *models.FeedbackRecord { + rec, err := repo.Create(ctx, &models.CreateFeedbackRecordRequest{ + SourceType: "formbricks", + FieldID: "q1", + FieldType: models.FieldTypeText, + ValueText: &valueText, + TenantID: tenantID, + SubmissionID: testTenantID("sub"), + }) + require.NoError(t, err) + + return rec + } + + recA1 := makeText(tenantA, "one") + recA2 := makeText(tenantA, "two") + + // Insert-only River client (no queues/workers) just to satisfy the worker's + // ClientFromContext seam; the per-record inserts skip the unknown-kind check. + riverClient, err := river.NewClient(riverpgxv5.New(pool), &river.Config{}) + require.NoError(t, err) + + svc := service.NewFeedbackRecordsService(repo, nil, "", nil, nil, "", 0) + worker := workers.NewTenantTranslationBackfillWorker(svc, 3) + + job := &river.Job[service.TenantTranslationBackfillArgs]{ + JobRow: &rivertype.JobRow{Attempt: 1, MaxAttempts: 1}, + Args: service.TenantTranslationBackfillArgs{TenantID: tenantA}, + } + require.NoError(t, worker.Work(rivertest.WorkContext(ctx, riverClient), job)) + + var count int + + err = pool.QueryRow(ctx, ` + SELECT count(*) FROM river_job + WHERE kind = 'feedback_translation' + AND args->>'feedback_record_id' = ANY($1)`, + []string{recA1.ID.String(), recA2.ID.String()}).Scan(&count) + require.NoError(t, err) + + assert.Equal(t, 2, count, "the worker enqueues one feedback_translation job per stale record") +} + +func TestTenantTranslationBackfillWorker_Timeout(t *testing.T) { + worker := workers.NewTenantTranslationBackfillWorker(nil, 3) + + assert.Equal(t, 5*time.Minute, worker.Timeout(nil), "per-tenant fan-out timeout") +}