Tags: formbricks/hub
Tags
fix: reset leader session state and withdraw backlog gauge on shutdow… …n (ENG-1670) (#116) * fix(enrichment): reset leader session state, clear gauge on shutdown (ENG-1670) Review follow-ups on the leader election and backlog gauge: - The leader's `SET idle_session_timeout` leaked onto the pooled connection. SET is session state and the pool hands that exact backend to unrelated callers afterwards, so Postgres could eventually terminate some other component's idle connection. release() now RESETs it before returning the connection. Pinned by a MaxConns=1 test that fails with "30min" if the reset is removed. - The gauge was not withdrawn on graceful shutdown -- the most frequent handover of all. Close released the advisory lock while the values stayed, so the meter provider's final export published a backlog this process no longer owned. Added the matching defer. - Corrected the alerting guidance, which the withdraw-on-demotion change had inverted: a failed poll now removes the series rather than freezing it, so the symptom is a MISSING gauge and a value-based staleness rule would not catch it. Updated the interface doc, the metric description and the escalated log. - Fixed a comment that contradicted migration 020 about the 016 index predicate, and one left over from the transaction-scoped lock. - Made the withdraw test load-bearing: it counted zero iterations and would have passed even with a wrong metric name. - Documented why the 020 index rebuild accepts a brief no-index window (its only consumer is the manual backfill command, which degrades to a seq scan) rather than adding a temp-name-and-rename dance. * fix(enrichment): never return an uncleaned leader connection to the pool (ENG-1670) Review follow-ups on #116: - release() shared one deadline between the advisory unlock and the idle-timeout RESET, so a slow unlock could consume the budget the reset needed, and both errors were discarded before handing the connection back -- exactly the leak the reset exists to prevent. Each statement now gets its own detached deadline, both errors are checked and logged, and if either fails the connection is taken out of the pool (Hijack + Close) instead of being reused: ending the session releases whatever the statements could not undo. Losing one pooled connection is cheaper than leaking a lock or a timeout onto a shared one. - The session-state test could have passed vacuously: tryAcquire sets the timeout best-effort and still grants leadership if the SET fails, so asserting a hard-coded "0" proved nothing on a server that ignores it. It now captures the original value, proves the server honours idle_session_timeout at all, and asserts release restores that captured value. - Register the leader's deferred Close before taking leadership so a failing assertion cannot leave the connection checked out and wedge db.Close(). The probe that verifies server support goes through the pool for the same reason, and uses set_config() because pgx's extended protocol rejects "SET ...; SELECT ...". * fix(enrichment): join the backlog poller before shutting down metrics (ENG-1670) Run returns the moment its context is cancelled and never joins runEnrichmentBacklogPoller, so the poller's deferred ClearEnrichmentPending raced Shutdown. Losing that race defeats the point of the clear: the meter provider's final collect-and-export publishes one last backlog reading for a process that has already lost leadership, which is the stale series the clear exists to withdraw. Track the poller with a done channel and join it in Shutdown, registered after the observability defer so LIFO runs the join first. A defer rather than an inline call, so the early-return error paths are covered too. The join is bounded by the shutdown context: a poller stuck in its leader cleanup must not hold shutdown past its budget, and a stale gauge sample is cheaper than a wedged shutdown. Postgres releases the session lock when the socket closes regardless. Waiting also fixes a second, quieter race: the leader's advisory-lock release and idle-timeout reset are best-effort with nothing awaiting them, so process exit could cut them off mid-statement. Only this poller is tracked. runRiverQueueDepthPoller has no clear-on-exit semantics -- it just stops updating, and its last reading is meant to be exported -- so joining it would add shutdown latency for no benefit. TestShutdownWaitsForEnrichmentBacklogPoller pins the ordering; deleting the join defer makes it fail with [metrics-exported metrics-shutdown], the gauge exported before the clear runs at all.
feat: add feedback-record count endpoints (tenant total + per taxonom… …y node) (#103) * chore: feedback records count api * fix: inline count endpoint query params in openapi spec The count operation $ref'd components/parameters/QueryParam* entries that never existed (the spec inlines all params; there is no components/parameters section). This broke Spectral lint, the Stainless SDK preview build, and the schemathesis contract test. Inline the 10 filter params, mirroring the list endpoint. * refactor: use named schema and typed response for count endpoint Promote the count 200 response to a named CountFeedbackRecordsOutputBody component (additionalProperties: false, count as int64) to match the sibling output bodies, and return a typed models.CountFeedbackRecordsResponse from the handler instead of an inline map — mirroring DeleteFeedbackRecordsByUser. Addresses CodeRabbit review feedback. * feat: add taxonomy per-node feedback-record counts endpoint Add GET /v1/taxonomy/runs/{run_id}/record-counts, returning the feedback-record count for every visible node in a run so the taxonomy view can show totals per topic and subtopic. Each count is a subtree total (a branch sums its subtopics, the root reports the run total). Records attach to nodes indirectly via cluster memberships, and the (run_id, feedback_record_id) uniqueness constraint means a record belongs to one cluster per run — so per-cluster counts sum across a subtree without double counting. The repo runs a single GROUP BY over taxonomy_cluster_memberships and rolls the counts up the visible tree in memory. Tenant-scoped via GetRunForTenant (404 for another tenant), mirroring the tree endpoint. * refactor: dedupe shared feedback-record filter params in openapi spec The list and count endpoints repeated the same 10 filter query params verbatim (~90 lines), which would drift as filters are added. Extract them into components/parameters and reference from both; the resolved spec is unchanged. Also stop the count repository error from re-using the service-layer message (was 'count feedback records: count feedback records: ...'); the repo now names the DB op ('query feedback records count'), matching the repository convention. * refactor: count taxonomy node records with distinct-record SQL Replace the per-cluster count plus in-memory subtree rollup with a single recursive descendant-closure query using COUNT(DISTINCT feedback_record_id). The previous approach summed per-cluster counts, which was only correct while each cluster was referenced by exactly one node — an invariant the schema does not enforce. The SQL now counts distinct records per subtree, so a cluster shared by multiple nodes is never double counted. Cover the shared-cluster case in the integration test (two leaves referencing one cluster: the branch and root report the distinct total, not the sum), and decode the feedback-count responses into the typed model.
feat: add value_id to feedback records for stable option identity (#104) * feat(feedback-records): add value_id column and partial index Add a nullable value_id VARCHAR(255) to feedback_records for stable option identity (ENG-1671): the source system's option id, stored alongside the human-readable value_text. The migration runs without a transaction so it never holds a long lock on this high-write table: the ADD COLUMN is metadata-only, and the lookup index is built CONCURRENTLY. The index is a partial composite on (tenant_id, field_id, value_id) WHERE value_id IS NOT NULL — it serves the "all records for option X" lookup and the value_id list filter while staying small and off the ingestion write path for the common text-record (value_id NULL), mirroring the table's other sparse-column partial indexes (sentiment, emotions). * feat(feedback-records): accept, return, and filter records by value_id Wire value_id through the API (ENG-1671): it can be set on create and update, is returned on retrieve/list, and is usable as a list filter (the "update all records for option X" flow). Hub treats it as an opaque string (max 255, no null bytes); it is NULL for free-text/non-choice answers and sources without option ids, where value_text remains the fallback identity. - model: ValueID on FeedbackRecord + create/update requests and the list filter; included in FieldsChangedFrom/ChangedFields so a value_id edit fires the update event like the other value fields. - repository: value_id in the shared column list + scan, the Create insert, buildUpdateQuery (a plain assignment — not an enrichment output, so it is never eager-cleared), and the list filter conditions. - taxonomy: value_id added to the parallel node-records column list that feeds the shared scan. - openapi: value_id on the create/read/update schemas and as a list query param. Covered by repository round-trip, list-filter, model change-detection, update-query, and handler validation tests. * fix(feedback-records): make list-filter placeholder numbering order-independent buildFilterConditions derived each $N from a manual argCount that the trailing Until branch never incremented. It was harmless only because Until is appended last and callers recompute the keyset/limit params from len(args) — but any filter added after Until would have bound the wrong placeholder. Derive each placeholder from len(args)+1 at the point of appending instead, so a condition's $N always equals its argument's 1-based position regardless of order, and the whole class of counter-drift bugs is gone. Locked by TestBuildFilterConditions_PlaceholdersMatchArgs, which asserts placeholder/arg alignment with every filter set.
feat(observability): enrichment round-2 (event correlation, HNSW-degr… …adation gauge) + perf nits (#100) * perf(embeddings): compute similarity score in Go; build embedding input once Two free wins surfaced by the Fable-5 review, no behavior change: - The nearest/after-cursor queries selected both `(e.embedding <=> $1) AS distance` and `(1 - (e.embedding <=> $1)) AS score`, evaluating the cosine-distance operator twice per row. Drop the score expression and derive score = 1 - distance in Go after the scan (distance is still what the query orders and cursors by). The semantic-search integration test already asserts score == 1 - distance. - The embedding provider's create path called BuildEmbeddingInput once for the empty-text check and again inside the dedupe hash. Build it once and reuse it; the now-single-caller embeddingValueTextHash helper is inlined to hashContent. * feat(observability): gauge for HNSW iterative-scan degradation When the server rejects hnsw.iterative_scan (pgvector < 0.8), the repo latches the fallback and logs once — after which nearest-neighbor recall is silently capped at ef_search for the process lifetime, invisible to monitoring once the log scrolls. Expose the latch as EmbeddingsRepository.IterativeScanDegraded() and register an observable gauge (hub_hnsw_iterative_scan_degraded, 0/1) that polls it, wired alongside the search handler so it is only registered when embeddings are enabled. No-op when metrics are disabled. * feat(observability): carry originating event id in enrichment job args Enrichment jobs recorded no link back to the domain event that enqueued them, so a job in the River UI (or its logs) could not be correlated to the event, and an event could not be traced to the jobs it fanned out. Add an EventID field to all four enrichment job args (sentiment, emotions, translation, embedding), populated from event.ID at enqueue — threaded through the generic provider's buildArgs hook and set directly on the embedding provider's insert. EventID is deliberately NOT part of the River unique key (no river:"unique" tag): it is correlation metadata, must not affect content dedupe, and is uuid.Nil for backfill jobs, which have no originating event. Adding the field is a backward-compatible JSON change (older in-flight jobs decode with a zero EventID). Worker-side logging of it follows in the next commit. * feat(observability): log the originating event id in enrichment workers Build on the EventID now carried in job args: emit it on the worker log lines so a job's logs correlate to the event that enqueued it (and, with feedback_record_id, to the record). In the generic enrichment worker this is done once by binding a slog.Logger with feedback_record_id (+ event_id when the eventID hook is set) and threading it through Work and its error helpers — which also removes the feedback_record_id key duplicated across ~12 call sites. Sentiment, emotions, and translation wire the eventID hook; the standalone embedding worker adds event_id to its record-gone/get-failed/stored lines (its remaining lines fold in with the scaffold migration, ENG-1643). event_id is uuid.Nil for backfill jobs, which unambiguously marks a non-event-driven origin. * refactor(observability): complete event_id coverage on the embedding path Addresses the #100 review: the standalone embedding worker logged event_id on only 3 of its ~10 lines, while the generic worker logs it on every branch. Bind a slog.Logger once in Work with feedback_record_id + event_id and thread it through handleEmbedError, handleSetEmbeddingError, and handleEmptyText — so every embedding outcome (rate-limit snooze, API failure, superseded/purge write skips, clear, skip) now carries event_id, and the feedback_record_id key is no longer repeated across 10 call sites. Also add event_id to the two enqueue-side provider debug lines that had feedback_record_id but not event_id (enrichment "skip, not enabled for tenant"; embedding "skip, no value_text on create"), matching their sibling log lines. * test(observability): lock event-id propagation and the iterative-scan gauge Addresses the #100 review's test-coverage gap: - Each enqueue provider test (sentiment, emotions, translation, embedding) now asserts the enqueued job args carry EventID == event.ID, so a regression that dropped EventID in any buildArgs closure or the embedding insert fails loudly (previously it would pass silently — the field was set but never asserted). - New RegisterHNSWIterativeScanGauge test: nil meter / nil callback are no-ops, and via a manual-reader meter the gauge reports 0 when the latch is healthy and 1 once degraded (the callback is re-polled per collect). * feat(observability): counter for enrichment outputs cleared on edit Round-1 made the eager-clear observable via a log line; this adds the metric the Fable review (A3) asked for so the clear RATE is queryable, not just grep-able. New hub_enrichment_outputs_cleared_total{output=sentiment|emotions|translation} increments once per output an edit's eager-clear nulls. The label set is bounded (NormalizeClearedOutput → "other" for anything unexpected). Wired through a new EnrichmentClearMetrics collector + aggregate field; the service records it via a SetEnrichmentClearMetrics setter, set only on the API instance (the eager-clear fires on UpdateFeedbackRecord) and left unset — a no-op — on worker/backfill. Tests cover the counter (emit + label normalization) and the service wiring (a value_text edit that nulls sentiment records one "sentiment" clear). * docs: refresh AGENTS.md for the enrichment framework The guidance drifted as the enrichment work landed. Update it to match the tree: - Structure: list the backfill commands (cmd/backfill-*), the four worker enrichment pipelines, and the internal packages that had appeared since (observability, llm/openai/googleai, datatypes, huberrors) + pkg/cursor, pkg/embeddings. - Commands: `make run` starts both api and worker (was described as api-only); add the run/build backfill targets. - New "Enrichment Framework" section: points at the shared classify scaffold (registry, classifyStructured, EnrichmentProvider, generic enrichmentWorker, shared metrics) and the thin per-type recipe, so a new enrichment reuses the scaffold instead of copy-pasting one — plus the read-only/eager-clear and supersession invariants. - Testing: split unit (internal, no DB) from integration (tests/, needs a pgvector test_db); note the tenant-isolation alternate-path test requirement. - Config: point at the enrichment/OTel env groups documented in .env.example. * fix(observability): correlate the translation short-circuit log line The translation worker's "source already in target language, copying value_text" short-circuit logged only feedback_record_id — the one main-line in the translation path still missing the event_id that every other worker log line now carries. Thread the event id (already present on the job args) into translate() and log it, matching the generic worker's field order. Completes event_id coverage for the translation path; no behavior change.
feat(sentiment): add sentiment enrichment for open-text feedback (#94) * feat: add sentiment + sentiment_score to Hub feedback records Adds the two server-generated sentiment-enrichment fields to feedback_records (ENG-1529): sentiment (6-level label very_negative..very_positive + mixed) and sentiment_score (signed polarity, -2..2), both nullable until enriched. Migration 014 adds the columns with CHECK constraints + a partial composite index; the model gains a typed SentimentValue enum and the two fields; the read path (feedbackRecordColumns + scanFeedbackRecord) and the OpenAPI feedback-record schema carry them. Also relocates scanFeedbackRecord next to its feedbackRecordColumns const (it lived in taxonomy_repository.go) so the SELECT and scan order stay in lockstep. * feat(sentiment): add SENTIMENT_* provider config gate Sentiment enrichment (ENG-1529) is disabled unless SENTIMENT_PROVIDER and SENTIMENT_MODEL are both set, mirroring the provider+model gate used by embeddings and translation (no separate enable flag). Add SentimentConfig with an Enabled() helper, document the vars in .env.example, and cover the gate in config tests. * feat(llm): add structured-JSON completion to the LLM clients Add a provider-agnostic structured-output seam for enrichments that need a typed result (sentiment, and later emotion), with no existing pattern to reuse: - internal/llm: an SDK-neutral Schema/Property spec so a structured request is described once and rendered to each provider's native schema type. - openai.Client.CompleteJSON: OpenAI Structured Outputs (response_format json_schema, strict) — a closed object with every property required. - googleai.Client.CompleteJSON: Gemini responseMimeType=application/json with a typed responseSchema. Both return the raw JSON string; the service layer owns the prompt, schema, and parsing/validation (numeric bounds are validated post-parse, since OpenAI strict mode does not support them). Factor the shared 429->RateLimitError mapping and completion-text extraction out of Translate so both calls share one error path. Tested via httptest against each real SDK: the request carries the strict schema and the JSON response parses. * feat(sentiment): add sentiment classification client and factory Add the SentimentClient seam (Classify -> {label, score}) and a provider factory mirroring the translation factory (openai / google / google-gemini registry, ValidateSentimentConfig, NewSentimentClient), driven by SENTIMENT_* config. The client owns the contract: it builds the prompt (six-level scale + mixed, optional source-language hint), passes the structured-output schema to the LLM client's CompleteJSON, then parses and validates — unknown label or unparseable response is rejected, and the score is clamped to [-2, 2] so a stray value still satisfies the DB CHECK. The schema enum derives from models.SentimentValues, now an ordered slice that also backs IsValid, so the label set has one in-Go source. Extend SentimentConfig with BaseURL / GoogleCloud{Project,Location} for full provider parity, and document them in .env.example. * feat(sentiment): add sentiment pipeline metrics Add SentimentMetrics (jobs-enqueued / provider-error / outcome / worker-error / duration) mirroring the embedding and translation collectors, with bounded reason and status allow-lists, and wire it into the aggregate Metrics bundle. The provider and worker depend on this interface (nil when metrics disabled). * feat(sentiment): add sentiment enqueue provider and job args SentimentProvider enqueues one feedback_sentiment job per eligible record event, mirroring the embedding provider (the simpler, target-free shape): text fields with non-empty value_text on create, or a value_text change on update (enqueued even when now empty so the worker can clear). Dedupe is by (record, value_text hash); unlike translation the source language is not part of the hash or the re-enqueue trigger, since sentiment is classified directly from the text. Enqueue failures are logged and counted, never blocking ingestion. * refactor(workers): extract shared rate-limit snooze helper The translation worker's rate-limit snooze (provider 429 -> bounded JobSnooze, capped by a max snooze window) applies verbatim to any LLM-backed enrichment worker. Lift it to a package-level rateLimitSnoozeDelay(err, jobCreatedAt) helper (with the bound constants) so the upcoming sentiment worker reuses it instead of duplicating the logic. Behavior is unchanged; the translation worker and its test now call the shared helper. * feat(sentiment): add sentiment worker and SetSentiment write FeedbackSentimentWorker fetches the record, classifies value_text via the SentimentClient, and persists the result (or clears it when value_text is empty), mirroring the embedding worker's not-found/tenant-conflict handling and reusing the shared rate-limit snooze for provider 429s. Skips non-text fields and a record gone before write; a provider failure retries to the attempt limit. SetSentiment (repository + service) writes both columns under the shared tenant write lock with no domain event and — unlike SetTranslation — no supersession guard, since sentiment has no per-tenant target. The service clears on a nil label and rejects an unknown label or a label without a score; the column CHECKs are the final guard. * feat(sentiment): wire the sentiment pipeline end to end Gate the sentiment client, enqueue provider, and worker on cfg.Sentiment.Enabled() (SENTIMENT_PROVIDER+MODEL), mirroring the translation/embedding wiring: - hub-worker builds the client + a lean records service and registers the worker on the sentiments queue (cfg.Sentiment.MaxConcurrent). - hub-api registers the worker/queue so it can enqueue at insert time, and registers the SentimentProvider off the River client (no tenant-settings dependency, unlike translation). Add SENTIMENT_MAX_CONCURRENT / SENTIMENT_MAX_ATTEMPTS (defaults 5 / 3) and document them. With sentiment unset, nothing is registered and no jobs run. * test(sentiment): add worker pipeline integration test Drive FeedbackSentimentWorker end to end against Postgres with a fake classifier: it classifies and persists the label+score through the real SetSentiment write, clears a stale sentiment when value_text is empty (without calling the provider), and SetSentiment on a missing record returns NotFound. * fix(sentiment): mark Gemini enum fields format:enum so they are enforced The genai schema contract enforces an enum only when the field is also marked format:"enum"; geminiResponseSchema set Enum without it, so on the Gemini path the sentiment labels were a hint, not a constraint — an out-of-enum label would fail validation and waste a retry. OpenAI's strict json_schema path was already enforced. Also: document that the sentiment model must support structured output (.env.example), and correct migration 014's comment to say the label/score relationship is steered by the prompt, not enforced by the worker. * test(sentiment): dedupe client-test helper, guard schema/parser coupling - Extract the duplicated mustMap JSON-body assertion into a shared internal/llm/llmtest package (imported only from _test.go, so testify stays out of the production import graph); the openai and googleai client tests use it. - Add a test that builds a structured response keyed by the sentiment schema's own property names and parses it, so renaming a schema property without updating sentimentResponse's json tag fails loudly (verified via mutation). * fix(sentiment): make the 014 migration lock-safe on feedback_records feedback_records is the primary, high-write table, so avoid holding a long ACCESS EXCLUSIVE lock during the migration (it previously added two validated CHECK constraints in-transaction, forcing a full-table verification scan under ACCESS EXCLUSIVE). Switch to the NO TRANSACTION + CONCURRENTLY pattern already used by the index migrations (005/006/009): - add the CHECKs NOT VALID (instant; immediately enforced for new/updated rows), then VALIDATE them as separate auto-committed steps that take only SHARE UPDATE EXCLUSIVE, so reads and writes proceed during the scan; - build the partial index CONCURRENTLY. ADD COLUMN of a nullable column with no default stays metadata-only. Verified: down+up re-applies cleanly, both constraints end up convalidated, and the sentiment integration tests pass against the rebuilt schema. * feat(sentiment): gate enrichment on a per-directory enable setting ENG-1529 requires sentiment to honor a per-directory AI-enrichment switch, not just the deployment-level provider/model gate. Add a tenant-scoped sentiment_enabled setting and enforce it on the enqueue path: - tenant_settings gains sentiment_enabled (stored, PUT, PATCH/RFC-7396). It is tri-state and defaults to enabled (opt-out): a tenant is enriched unless it has explicitly switched sentiment off. EnrichmentSettings.SentimentEnrichmentEnabled() centralizes that default. - SentimentProvider now resolves the tenant's setting (after the cheap eligibility checks) via a shared tenant-settings cache and skips disabled tenants; a settings read failure skips rather than enqueuing blindly. - Translation and sentiment now share one tenant-settings cache, evicted on any settings write so a toggle takes effect immediately. - OpenAPI documents the new field on the settings schemas. Tested: service PUT/PATCH/null + key-tag pinning, provider gate (disabled skips, enabled/absent enqueue, read-error skips), and a full HTTP settings round-trip. * feat(sentiment): default to neutral when the model is unsure ENG-1529 specifies neutral (score 0) as the default when the classifier is uncertain. Make that explicit in the prompt. * refactor(observability): share one implementation across enrichment metrics The embedding, translation, and sentiment metrics were three ~118-line files of near-identical logic (the same five instruments, record methods, and status normalization), differing only in metric names and the bounded reason/status label sets. That copy-pasted logic was the real duplication risk — a fix or new label had to be made in three places. Extract one enrichmentMetrics implementation (instrument creation, the shared Record* methods, normalization) parameterized by a per-pipeline spec. Each pipeline keeps its public interface and constructor but is now a 2-method adapter (its differently-named outcome/duration methods) over the shared impl, so the three interfaces and every caller (workers, providers, wiring, the aggregate bundle) are unchanged — the change is confined to the observability package. Left as-is on purpose: the client factories and worker error-handlers look parallel but diverge per pipeline (client type, embedding DocPrefix, translation supersession, the google-vertex alias); collapsing them would need long parameter lists / generics-over-config that read worse than the parallel code. * fix(sentiment): enforce a closed-object schema on both LLM providers CodeRabbit: the Gemini path used ResponseSchema (the OpenAPI subset), which does not enforce additionalProperties:false, so extra keys could pass the provider boundary — unlike the OpenAI strict path. Switch Gemini to ResponseJsonSchema (standard JSON Schema) and render the schema once via a shared llm.Schema.JSONSchema() builder used by both clients. Both providers now enforce the same closed-object contract, and the OpenAI-map vs Gemini-genai.Schema duplication is gone (one builder, tested in internal/llm). * fix(sentiment): tighten persistence and parsing integrity Address several CodeRabbit data-integrity findings: - SetSentiment now returns NotFound when the UPDATE affects zero rows (record deleted after the tenant lock), matching its doc contract and the worker's benign-skip path instead of reporting a phantom success. - sentimentResponse.Score is now *float64: a response omitting score is rejected as ErrSentimentResponseInvalid rather than silently persisted as 0. - migration 014 adds a paired-nullability CHECK (sentiment and sentiment_score set together or both NULL) as defense-in-depth against a half-populated write, and uses the documented lowercase -- +goose up/down annotations. - models.SentimentValues is no longer an exported mutable slice: it is an unexported canonical slice exposed via SentimentValues() returning a copy, so the label set can't be mutated out from under IsValid / the schema enum. * fix(workers): bound the rate-limit snooze to the max window CodeRabbit: the snooze window was only checked before choosing the delay, so a job still inside maxRateLimitSnoozeWindow could be re-queued for another full snooze and overshoot the cap by up to maxRateLimitSnooze. Also reject the snooze when elapsed + the chosen delay would exceed the window, so a rate-limited job falls through to the normal-failure/backfill path on schedule. Covered by a new overshoot case in the snooze test. * chore(sentiment): address review nits - embeddings worker-error metric description is provider-neutral (was openai-specific but the constructor is multi-provider). - .env.example lists SENTIMENT_PROVIDER=google (a supported provider that was missing). - sentiment enqueue test asserts the River UniqueOpts (ByArgs + ByPeriod), pinning the dedupe contract. - the integration not-found case now drives worker.Work end to end (GetFeedbackRecord -> NotFound -> benign skip), alongside the repository-level SetSentiment NotFound check. * refactor(sentiment): normalize sentiment_score to the -1..1 polarity range Rescale the signed sentiment_score from -2..2 to the conventional -1..1 polarity range (Google Cloud NL / TextBlob / VADER) across the model bounds, prompt anchors, structured-output schema description, the 014 migration CHECK, OpenAPI min/max, .env.example, and the tests. * fix(sentiment): record retriable worker failures as retry, not failed_final The get-record and generic-write error paths recorded the job outcome as failed_final on every attempt, but River retries the job while attempts remain, so hub_sentiment_outcomes_total{status="failed_final"} overcounted final failures. Record retry vs failed_final by attempt (as the classify and tenant-write-conflict branches already do) and surface final_attempt in the error log. Adds table tests pinning both paths across a non-final and the final attempt. * fix(sentiment): fail open on a settings-read error; make the worker the gate The enqueue provider dropped the job when the per-tenant settings read failed, permanently losing enrichment for events ingested during a transient tenant_settings/cache outage (the event path is best-effort, not retried). Fail open instead: the provider enqueues on a settings-read error (still logging and recording the provider-error metric), and the sentiment worker re-checks the per-directory switch as the authoritative gate before the LLM call. A read error in the worker retries (so a fail-open enqueue is never lost); a tenant that turned sentiment off is skipped without classifying or clearing, matching what the provider gate does when settings are readable. Wires an uncached tenant-settings reader into the worker in both hub-worker (where jobs run) and hub-api (insert-only), adds the settings_read_failed worker metric reason, and covers the disabled/read-error/fail-open paths with tests.
feat: serialize tenant-owned writes against tenant data purges (#87) * feat: serialize tenant-owned writes against tenant data purges Tenant data purges now take an exclusive transaction-scoped advisory lock per tenant while every tenant-owned mutation (feedback records, webhooks, embeddings, taxonomy) holds the same key in shared try-lock mode inside a single repository-level helper. Writes for a tenant under purge fail fast with a retryable 409 (code tenant_write_conflict); the purge waits up to TENANT_PURGE_LOCK_TIMEOUT_SECONDS (default 5s) for in-flight writes to drain and then returns the same retryable conflict. Other tenants are unaffected. ID-based mutations now resolve the row's tenant inside the transaction and mutate with tenant-scoped WHERE clauses; the GDPR delete-by-user locks every spanned tenant; tenant-changing webhook updates lock both tenants. Workers skip or retry cleanly on conflicts. Closes ENG-1013 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: consolidate tenant write lock helpers and interfaces * chore: address PR comments * chore: address pr issues --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
PreviousNext