feat: expose per-tenant enrichment status and backlog metric (ENG-1670) - #111
Merged
Conversation
Expose per-tenant, data-derived enrichment progress so the UI can surface an
"in progress" indicator, mirroring the embeddings UX (eligible vs done counts,
not queue depth).
GET /v1/enrichment-status?tenant_id= returns {enabled, eligible, done} per
enrichment (translation, sentiment, emotions) counted over feedback_records:
- eligibility is text-with-content (field_type='text', btrim charset matches the
workers' TrimSpace), gated by the per-tenant switch (sentiment/emotions) or a
resolvable target language (translation);
- "done" for translation means the stored lang key equals the effective target
(stale translations are not done);
- the deployment-level provider/model gate is overlaid in the service.
tenant_id is required and validated; the response is counts-only (no record
ids/content); the query is parameterized and strictly tenant-scoped.
Emit hub_enrichment_pending_records{enrichment}, an async gauge of
eligible-but-unenriched feedback records per enrichment type, summed across all
tenants. Unlike the transient River queue-depth gauge this is a data-derived
completeness signal that persists across queue drains.
A background poller in the API process refreshes it on a conservative interval
with a per-iteration query timeout (the aggregate is a full-table scan), gated on
metrics being enabled and reporting only deployment-configured enrichments. The
per-tenant enable gates still apply in SQL, so a disabled tenant never inflates
the backlog. The gauge is labeled by enrichment type only — never tenant_id — to
keep cardinality bounded; metrics are OTLP-push, so it never reaches the public API.
Register the endpoint (tag "Enrichment Status", operationId get-enrichment-status → enrichmentStatus.retrieve) plus the EnrichmentStatusOutputBody / EnrichmentTypeStatus schemas, so Stainless regenerates the SDK and API reference. Validated with spectral (make lint-openapi).
- DRY the count SQL: the per-tenant and aggregate queries now share a single
enrichmentCountSelect / enrichmentCountFrom so the six FILTER predicates can't
drift; the effective-target placeholder is unified to $1 in both.
- Document that the per-tenant scan is served by idx_feedback_records_tenant_field_type
(tenant_id, field_type) — the existing composite index already covers it, so no new
index is needed.
- Name the metric-label values (observability.EnrichmentType{Translation,Sentiment,
Emotions}) instead of string literals in the poller.
…-1670) The aggregate backlog gauge is a slow-moving trend signal, so a full-table scan every 60s over the high-write feedback_records table is more often than needed. Widening to 5m cuts the shared-DB load 5x and keeps the MVCC snapshot the scan holds short-lived relative to VACUUM. The poller still only runs when metrics are enabled, and each scan is bounded by a 30s timeout.
WalkthroughAdds a tenant-scoped enrichment status API covering translation, sentiment, and emotions. Introduces repository queries for eligible and completed enrichment counts, service logic for deployment and tenant settings, and OpenAPI schemas. Adds aggregate pending-enrichment gauges with a periodic application poller. Updates application and test-server wiring, adds handler, service, observability, and Postgres integration tests, and bumps two Go module dependencies. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✱ Stainless preview buildsThis PR will update the ✅ hub-typescript studio · code
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
govulncheck flags GO-2026-5970 (infinite loop on invalid input in golang.org/x/text, fixed in v0.39.0), failing the Code Quality gate. The vulnerable paths are pre-existing (webhook sender, pgx pool, embedding input) and unrelated to this PR, but the scan runs module-wide. Bump to v0.39.0; govulncheck is clean afterwards. go mod tidy also pulled x/sync v0.20.0 -> v0.21.0.
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/api/app.go`:
- Line 720: Normalize the fallback language passed in the poller’s configuration
by trimming a.DefaultLanguage before assigning it to defaultLang in the
NewEnrichmentStatusService setup. Keep the normalized value consistent with the
service’s existing DefaultLang handling so SQL and API translation-status
calculations use the same target.
In `@internal/repository/enrichment_status_repository.go`:
- Around line 71-86: Update both countEnrichmentStatusSQL and
countEnrichmentBacklogAggregateSQL to add fr.field_type = 'text' to their outer
WHERE clauses, preserving the tenant_id filter for the tenant-scoped query and
adding the predicate to the aggregate query so both scans visit only text
records.
In `@openapi.yaml`:
- Around line 1385-1434: Update the responses for the get-enrichment-status
operation to explicitly declare a 503 Service Unavailable response for the
unconfigured enrichment status service condition. Match the existing 503
response structure and ErrorModel reference used by analogous feature-gated
endpoints, while preserving the current 200, 400, 401, and default responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad5d8f92-e24d-4cb3-adf3-c0d7f2534e38
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
cmd/api/app.gocmd/api/app_test.gogo.modinternal/api/handlers/enrichment_status_handler.gointernal/api/handlers/enrichment_status_handler_test.gointernal/models/enrichment_status.gointernal/observability/aggregate.gointernal/observability/enrichment_backlog.gointernal/observability/enrichment_backlog_test.gointernal/observability/names.gointernal/repository/enrichment_status_repository.gointernal/service/enrichment_status_service.gointernal/service/enrichment_status_service_test.goopenapi.yamltests/enrichment_status_test.go
- repository: hoist `field_type = 'text'` into the outer WHERE of both count queries so the (tenant_id, field_type) index narrows the scan to text rows instead of scanning all rows and discarding non-text inside the FILTERs. Redundant with the per-column predicate, so no count changes (EXPLAIN confirms index access; integration tests still green). - api: trim the poller's fallback language for symmetry with NewEnrichmentStatusService so the endpoint and the backlog gauge resolve the same target (config already canonicalizes it; defensive). - openapi: document the 503 the handler returns when the service is unwired, matching the other feature-gated endpoints.
…670) Follow-ups from a final re-review: - Broaden the eligibility trim to the full ASCII whitespace set (add VT/FF -> E' \t\n\v\f\r') and correct the comment, which previously overclaimed an exact match with the workers' Go strings.TrimSpace gate. TrimSpace also strips exotic Unicode whitespace (NBSP, U+3000, ...), so a value composed entirely of those remains a rare eligible-but-never-done edge -- now documented and accepted, matching the existing backfill queries rather than pretending otherwise. - Add a DB-level test that the emotions per-tenant switch gates eligibility (previously only the sentiment switch was covered against Postgres).
Dhruwang
reviewed
Jul 27, 2026
Dhruwang
reviewed
Jul 27, 2026
BhagyaAmarasinghe
requested changes
Jul 27, 2026
…st plan note (ENG-1670) Review follow-ups: - Multi-replica: production runs several API replicas per region and each was repeating the same cross-tenant scan every tick and publishing the same global gauge (multiplied DB load, duplicate series a sum would over-count). The poll now runs under a non-blocking transaction-scoped advisory lock, so exactly one replica scans per tick; losing the race is a normal skip, not an error. Reuses the pg_try_advisory_xact_lock idiom already used elsewhere in the package. - Alerting: a failing poll left the gauge frozen at its last value, which reads as a healthy steady backlog on a dashboard. Failures now increment hub_enrichment_backlog_poll_errors_total and escalate warn -> error after three consecutive misses. - Correct the aggregate query's plan comment: it canNOT use the (tenant_id, field_type) index (tenant_id is the leading column and the aggregate has no tenant predicate), so it seq-scans by design. Documented why that is the right trade-off rather than adding a partial index that would tax the ingest path. Adds DB-backed coverage for the leader election and the poll-error counter.
…p (ENG-1670) Emotions could never reach "done". A successful classification that detects no emotion is stored as NULL (the 015 CHECK rejects the empty array), which is the same value that means "never classified", so those records counted as pending forever and the backlog gauge could not drain. Adds emotions_classified_at (migration 020) to record completion independently of the labels found: - SetEmotions stamps it (an empty result is still a completed classification); the new ClearEmotions clears both, for the content-is-gone path. - The eager-clear on a value_text edit drops the marker with the labels. - "done" is (emotions_classified_at IS NOT NULL OR emotions IS NOT NULL); the second arm covers rows enriched before the column existed, so the migration needs no bulk UPDATE of the primary write table. - The classify backfill now skips already-classified rows instead of re-sending every classified-empty record to the provider on each run. The marker is processing state and stays out of the API surface, so no response shape, OpenAPI or SDK change. Sentiment and translation are unaffected: their successful results are single-valued and always non-NULL (verified). Also replaces the previous backlog-poller advisory lock, which did not do what it claimed. It was transaction-scoped, so it was held only for the ~seconds a scan takes, while replicas tick on independent unsynchronized schedules -- they almost never collided, so neither the duplicated DB load nor the duplicate series was actually prevented, and a collision on a replica's first poll left it exporting no series at all until the next tick. Leadership is now a sticky session-scoped lock held on a dedicated connection for the process lifetime: exactly one replica scans and exports, the series stays put instead of flapping, followers never pin a connection, and Close hands over promptly on shutdown. Also stops counting a shutdown-cancelled scan as a poll error, which would otherwise fire the staleness alert on every rolling deploy.
…bound zombie leader (ENG-1670) Three defects found re-reviewing the leader election and the emotions marker: - A demoted leader kept exporting forever. The async gauge re-observes its stored values on every collection and nothing ever cleared them, so after a handover the old leader's frozen reading coexisted with the new leader's live one -- reintroducing exactly the double-count the election was meant to remove, and making the stale copy look like a permanently stuck backlog. The poller now withdraws its series whenever it is not the publishing leader (lost election or failed scan). Exporting nothing is honest; a frozen value is not. - Migration 020 had broken the 016 emotions-backfill index. That index is predicated on `emotions IS NULL` so rows leave it once enriched, keeping it near-empty; classified-empty rows now keep emotions NULL forever, so they would have been retained permanently -- an index growing without bound and a drained backfill forced to scan the whole retained set to find nothing. 020 now rebuilds it with the marker in the predicate, which also removes a post-filter (verified with EXPLAIN) and round-trips cleanly up/down. - A leader lost to node failure or a network partition held the session lock until TCP keepalives reaped the backend (hours by default), with no replica able to take over and no error to alert on. The leader session now sets a 30min idle_session_timeout -- far above the poll interval, so it can only fire on a session that has genuinely stopped polling. Also stamp the marker from the DB clock (CASE WHEN ... THEN NOW()) rather than the pod clock, matching updated_at beside it; extend the eager-clear drift guard to the new column; and release leadership via defer in the leader test so an early failure reports instead of wedging the suite on a checked-out connection.
…-1670) The SDK preview build flagged Model/Recommended for this schema: it is referenced three times (translation, sentiment, emotions), so without a model declaration the generated SDKs emit three structurally identical inline types instead of one shared one. Mirrors the existing taxonomy.run / taxonomy.node declarations.
… (ENG-1670) The camelCase model path failed Stainless generation (Name/NotSnakeCase, then a fatal). Model paths are snake_case; the existing taxonomy.run / taxonomy.node declarations are single lowercase words, so the convention was not visible there.
BhagyaAmarasinghe
self-requested a review
July 27, 2026 17:49
BhagyaAmarasinghe
approved these changes
Jul 27, 2026
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Gives the product a way to surface enrichment progress ("in progress" / "done") for a feedback directory, the way we already do for embeddings.
New endpoint:
GET /v1/enrichment-status?tenant_id=<id>returns, per record-level enrichment (translation, sentiment, emotions):{ "tenant_id": "…", "translation": { "enabled": true, "eligible": 500, "done": 480 }, "sentiment": { "enabled": true, "eligible": 500, "done": 500 }, "emotions": { "enabled": false, "eligible": 0, "done": 0 } }eligible= feedback records that qualify for the enrichment (open-text with content, and the enrichment is on for the tenant — a resolvable target language for translation, the per-directory switch for sentiment/emotions).done= how many have been enriched. The UI derives "in progress" aseligible - done(same as the embeddings banner).enabledfolds in the deployment-level provider/model gate; when it's false the counts are zero.Design note — this is data-derived, not queue-derived. The counts come from
feedback_records(mirroring how/v1/taxonomy/fieldsreportsrecord_count/embedding_count), not from the River queue. That choice is deliberate: it gives the progress-bar denominator the queue can't, and — importantly — it needs no changes to the ingest/enqueue path, so a burst of a few thousand incoming records is unaffected (no new index on the insert path, no per-job tagging). The per-tenant query is served by the existingidx_feedback_records_tenant_field_type.Also included
A schema change —
emotions_classified_at(migration 020). Review surfaced that emotions could never reach "done": a successful classification that detects no emotion is stored as NULL (the 015 CHECK rejects the empty array), which is the same value that means "never classified". Those records would have counted as pending forever and the backlog gauge could never drain. The new column records completion independently of the labels found:SetEmotionsstamps it (an empty result is still a completed classification); a newClearEmotionsclears both, for the content-is-gone path.value_textedit drops the marker with the labels, so an edited record correctly returns to pending.doneis(emotions_classified_at IS NOT NULL OR emotions IS NOT NULL)— the second arm covers rows enriched before the column existed, which is why the migration needs no bulk UPDATE of the primary write table.The marker is processing state and is deliberately kept out of
feedbackRecordColumns/ the API model, so there is no response-shape, OpenAPI or SDK change. Sentiment and translation are unaffected — their successful results are single-valued and always non-NULL (verified); emotions is the only enrichment whose success can legitimately be "nothing".Backlog metric.
hub_enrichment_pending_records{enrichment}— an OpenTelemetry gauge of the aggregate (cross-tenant) eligible-but-unenriched backlog, a durable completeness signal complementing the transient River queue-depth gauge. It only runs when metrics are enabled, is bounded by a 30s statement timeout, and polls every 5 minutes (a full-table aggregate over a high-write table, so we keep it infrequent). Labeled by enrichment type only — nevertenant_id— to keep cardinality bounded.Production runs several API replicas per region, so the poller is leader-elected: a session-scoped advisory lock held on a dedicated connection for the process lifetime means exactly one replica scans and exports the (global by definition) series. Leadership is deliberately sticky rather than per-scan — replicas tick on independent schedules, so a lock held only for the duration of a scan would almost never collide, reducing neither the DB load nor the duplicate series, while occasionally blanking a replica's series.
Failed refreshes increment
hub_enrichment_backlog_poll_errors_totaland escalate warn → error after three consecutive misses, because a failing poll otherwise leaves the gauge frozen at its last value — indistinguishable from a healthy steady backlog on a dashboard. A scan cancelled by shutdown is not counted (it would fire that alert on every rolling deploy).Dependency bump.
golang.org/x/text0.37.0 → 0.39.0 for GO-2026-5970, which was failing the govulncheck gate on unrelated pre-existing call paths.Out of scope (follow-ups): the Formbricks UI that consumes this, and a per-tenant
failedcount (permanent failures are already observable globally viahub_<type>_outcomes_total{status="failed_final"}).Linear: https://linear.app/formbricks/issue/ENG-1670
How should this be tested?
Automated
make tests— integration tests intests/enrichment_status_test.gocover the SQL against Postgres: eligibility (text + content only, whitespace-only and non-text excluded), translation done vs. stale (translation_lang_key= effective target), the sentiment and emotions per-tenant switches, the default-language fallback, strict per-tenant isolation, and the aggregate delta.Closehands over promptly.go test ./internal/...— service gating/zeroing, handler 400/503/200, the gauge and the poll-error counter, and that empty content takes the clear path rather than recording a classification.make migrate-validatefor migration 020.Manual (live progression)
Bring up
hub-api+hub-workeragainst a DB with translation + sentiment configured, set a target language for a tenant, create a few open-text records, then poll the endpoint. Config used:DATABASE_URL,API_KEY,TRANSLATION_PROVIDER/MODEL/_API_KEY,SENTIMENT_PROVIDER/MODEL/_API_KEY,TRANSLATION_DEFAULT_LANGUAGE.Created 3 text records, then polled:
doneclimbs from the partial state toeligibleas enrichment completes — that's the "in progress → done" signal the UI needs. (emotionsisenabled:falsehere only because that provider wasn't configured in the test env.) Also verified:401without the key,400whentenant_idis missing, and isolation (an unknown tenant returns zero counts).Checklist
Required
make buildmake tests(integration tests intests/)make fmtandmake lint; no new warningsgit pull origin mainmigrations/020_add_emotions_classified_at.sqlwith goose annotations and ranmake migrate-validate. NullableADD COLUMNwith no default, so it is metadata-only (instant, no rewrite, no long lock) and re-runnable; no data backfill by design (see the note above)Appreciated
make lint-openapi)docs/— the Hub API reference regenerates fromopenapi.yamlvia Stainless; the hand-authoredhub-api-docspages (metrics reference + core-concepts) are a separate follow-upmake tests-coveragefor meaningful logic changes