feat: cap the text sent to enrichment providers [ENG-3908] by xernobyl · Pull Request #151 · formbricks/hub

xernobyl · 2026-10-13T14:45:56Z

What does this PR do?

Hub accepts value_text up to 30,000 characters and sent all of it, uncut, to all four enrichments. That is ~7.5k tokens in English and far more in CJK, spent on every call and again on every edit. For embeddings it was worse than expensive: an embedding model refuses an over-long input, which since ENG-2375 is a terminal failure, so the record never got a vector and never became searchable — and the rejection was only discovered after paying for the call.

Each enrichment now sends at most its own configured number of bytes, checked before the provider call:

Enrichment	Over the cap	New env var (default)
Embeddings	trimmed to the first N bytes	`EMBEDDING_MAX_INPUT_BYTES` (`12000`)
Sentiment, emotions	trimmed to the first N bytes	`SENTIMENT_MAX_INPUT_BYTES`, `EMOTIONS_MAX_INPUT_BYTES` (`24000`)
Translation	skipped, recorded as a terminal failure	`TRANSLATION_MAX_INPUT_BYTES` (`24000`)

0 disables a cap; a negative value, or one below a single rune, is rejected at startup rather than silently meaning "unlimited".

Trimming is counted in a new hub_enrichment_text_trimmed_total{enrichment} (label-not-name, the convention the newer enrichment families use) — trimming is not a failure, so it is its own family rather than a reason on hub_enrichment_terminal_total. A translation skip is counted as hub_translation_worker_errors_total{reason="input_over_cap"}, so it can be told apart from a provider's own length rejection (the two share hub_enrichment_terminal_total{reason="length"}). Translation needs no new machinery: returning a huberrors.TerminalProviderError(TerminalReasonLength) reuses what ENG-2375/ENG-2376 already do end to end — no provider call, JobCancel, a durable length marker, failed_terminal in the status API, exclusion from the reconciler sweep, and a tenant-clearable retry. An edit that brings the text back under the cap heals the record with no cleanup.

Decisions worth a reviewer's attention

Bytes, not runes or tokens. No tokenizer is right for every deployment (OpenAI, Gemini and a self-hosted TEI endpoint each tokenize differently; neither SDK counts locally; counting provider-side costs a request on Gemini and does not exist on OpenAI). Bytes bound token count to within ~2x whichever script arrives, where the same budget in runes bounds it to within ~4x — the divergence the ticket worried about. service.TextCap documents this.
Per enrichment, not one global limit. Embeddings is the only pipeline with a hard model limit, so its cap must sit below the chat caps; the config surface is already per type.
The embedding cap is applied inside BuildEmbeddingInputFromValues, not at its call sites. That string is both what is sent to the provider and what the write-time supersession guard compares against; one place keeps them byte-identical (pinned by a test).
Translation refuses rather than trims, and the check runs after the copy-through short-circuits — a record with no source language is copied, sends nothing to a provider, and so must not be marked terminally failed.
The refused band is deliberate and is now documented. The 24,000-byte translation cap sits below the 30,000-rune ingest ceiling (up to ~90,000 bytes of CJK, ~120,000 for 4-byte runes), so accepted long-form answers are refused translation while sentiment, emotions and embeddings still trim and produce a result. That is the ticket's cost argument applied to CJK, but it has two consequences worth knowing: an over-cap translation is refused before any provider call, so retrying it is free, and raising TRANSLATION_MAX_INPUT_BYTES later does not clear the markers already written — a tenant clears them with POST /v1/tenants/{tenant_id}/enrichments/retry, after which the sweep re-runs at the new cap. Documented in .env.example, the env-var reference, the data-model note and translated-feedback.mdx.

Not in this PR

The semantic-search request's query also reaches the embeddings provider uncapped (bounded only by the 64 KiB body cap). That is a request-field bound, i.e. ENG-1652's scope, and fix(api): bound the remaining unbounded request fields [ENG-1652] #150 caps it at 4,096 characters.
A trimmed embedding has no per-record "truncated" flag: the OpenAPI description of a search result's value_text, and the docs note, now say that a vector may cover only the first N bytes rather than claiming the returned text is what was embedded. A per-record column would be a schema + spec + SDK change; worth it only if the product needs to show it.
ENG-1652's description still contains this work item as a paragraph. Worth de-duping in whichever ticket lands second.

Review outcomes (this round)

Three independent passes (correctness, contract, security) ran against the head, each verifying findings against the code rather than the base.

Fixed in e21104a:

openapi.yaml + search_handler.go + models/embeddings.go described a search result's value_text as "the text that was embedded" — false for exactly the records this ticket exists for, and it is the published contract that generates the API reference and SDK docs.
failed_terminal (OpenAPI) and the reconciliation section of the env-var reference described a terminal failure only as something the provider refused; the cap refuses before any call, and the "clear to retry after an edit" phrasing was misleading (an edit re-runs it by itself).
The data-model note claimed that raising EMBEDDING_MAX_INPUT_BYTES "embeds more of it". It does not: the backfill fills missing rows and never overwrites an existing vector, so already-embedded records keep the short vector until their rows are deleted and rebuilt. The note now names the lever that actually exists.
The trimmed counter's description now says it counts once per attempt; the embedding builder comment no longer claims a cap always leaves room for the label ahead of the answer.

Parked, not fixed here (review-followups.md): the four knobs are not surfaced in charts/hub/values.yaml; the search query has no cap and no rate limit; a tenant-settings write can fan out a translation backfill that races a create and enqueue a second translation job for the same record (reproduced locally — pre-existing, double provider spend, no dedupe on the event path).

Review outcomes (second round)

Fixed in d71cd6a:

An over-cap translation is refused before any provider call, but the shared worker counted it as translation_api_failed and logged "provider failed permanently" at Error. That would page whoever alerts on provider errors for a call that never happened. The refusal now wraps a sentinel the shared worker recognises: it counts under reason="input_over_cap" and logs a Warn saying no call was made. Terminal handling, the length marker and JobCancel are unchanged. The unit test asserts the new reason and fails without the fix (checked by reverting it).
models.FeedbackRecordWithScore said the vector covers "at least" the first N bytes. It is at most. The config floor's rationale was also tightened.

Review outcomes (Anshuman's review)

Fixed in 1d4741b: input_over_cap was exported as reason="other", because allowedTranslationWorkerReasons did not name it and the counting double in the worker tests skips the allowlist. The reason is now observability.WorkerReasonInputOverCap, the translation allowlist names it, and a new worker test runs the refusal through the real NewTranslationMetrics, asserting the exported series is exactly {input_over_cap: 1} (it fails with {other: 1} without the allowlist entry). AGENTS.md's input-cap recipe now names the allowlist.

Re-smoked on 1d4741b with a real OpenTelemetry Collector (0.115.0, debug exporter) receiving the worker's OTLP export: hub_translation_worker_errors_total{reason="input_over_cap"}=2 with no other and no translation_api_failed series, hub_enrichment_terminal_total{enrichment="translation",reason="length"}=2, and hub_enrichment_text_trimmed_total=2 for each of embedding, taxonomy_embedding, sentiment and emotions. Provider request sizes, markers, job states and /v1/enrichment-status match the earlier smoke.

Parked (pre-existing since ENG-2375): failure_marker_write_failed is missing from all three classify allowlists, so it too exports as other.

How should this be tested?

# Unit + integration (integration needs a pgvector Postgres)
DATABASE_URL="postgres://postgres:postgres@localhost:5433/test_db?sslmode=disable" \
  go test ./cmd/... ./internal/... ./tests/ -count=1     # all pass
make lint && make lint-openapi && make fmt && make build  # 0 issues, spectral clean

TestFeedbackTranslation_OverCapSkipIsTerminalAndDurable (tests/) drives the whole chain against Postgres: a 200-byte record with a 50-byte cap is refused with the provider never called, a terminal length marker lands, CountEnrichmentStatus reports failed_terminal=1/failed=0, ListPendingEnrichment does not return the record, editing the text back under the cap translates it again, and a record exactly at the cap is translated whole.

End-to-end smoke (~26 assertions, all passing; re-run on d71cd6a with a 13,500-byte Japanese record added — every cut lands on a rune boundary and no request exceeds its cap). hub-api + hub-worker + the workspace's ai-test-server as the OpenAI-compatible provider, with a capturing proxy in front of it recording the exact bytes of every request, an OTLP/HTTP sink reading the worker's metrics, and a 4,000-byte fr record plus a 300-byte one:

the embedding, sentiment and emotions requests arrive at exactly the configured caps (2,000 / 3,000 / 3,000 bytes) and nothing exceeds them; translation is never sent the over-cap text (only the 300-byte record's call exists);
the record stores sentiment/emotions and both embeddings (raw + taxonomy) — the case that used to be a terminal failure — while value_text_translated stays NULL;
one marker row, terminal='length', attempts=1, and the translation job is cancelled;
/v1/enrichment-status reports translation.failed_terminal=1, failed=0, done=1;
over 25s of 10-second sweeps the reconciler enqueued nothing for the terminally-failed record;
semantic search returns the record whose embedding came from a trimmed input;
after a PATCH down to 33 bytes the translation completes (value_text_translated set from the provider) and failed_terminal returns to 0;
the worker's OTLP export carries hub_enrichment_text_trimmed_total{enrichment=embedding|taxonomy_embedding|sentiment|emotions} and hub_enrichment_terminal_total{enrichment=translation,reason=length} — read back from a real protobuf export body, with no translation series in the trim counter (it skips, it does not trim).

Checklist

Required

Appreciated

If API changed: OpenAPI descriptions corrected (no shape change; /v1/enrichment-status already reports failed_terminal)
Updated docs in docs/ — env-var reference, metrics reference, data-model, translated-feedback, .env.example, and the enrichment-framework section of AGENTS.md
Ran make tests-coverage for meaningful logic changes

Hub accepts value_text up to 30,000 characters and sent all of it, uncut, to every enrichment. That is ~7.5k tokens in English and far more in CJK, on every call and again on every edit — and for embeddings it is worse than expensive: an embedding model refuses an over-long input, which since ENG-2375 is a terminal failure, so the record never gets a vector and never becomes searchable, while the provider's rejection is only discovered after paying for the call. Each enrichment now sends at most its own configured number of bytes (EMBEDDING_MAX_INPUT_BYTES default 12,000; TRANSLATION_/SENTIMENT_/EMOTIONS _MAX_INPUT_BYTES default 24,000; 0 disables the cap), checked before the provider call: - Embeddings, sentiment and emotions trim to the cap and count the cut in hub_enrichment_text_trimmed_total, because a vector or a label for the first part of a record beats none at all. - Translation skips and returns a terminal 'length' failure, because a translation of the first N bytes stored as if it were the whole text is wrong. This reuses the marker, status and retry machinery as-is: the record shows up as failed_terminal, the reconciler leaves it alone, and an edit that brings it back under the cap heals it with no cleanup. Bytes rather than runes or tokens: no tokenizer is right for every deployment (OpenAI, Gemini and a self-hosted TEI endpoint each tokenize differently, and counting provider-side costs a request on Gemini and does not exist on OpenAI), and bytes bound tokens within ~2x across scripts where the same budget in runes bounds them within ~4x. The embedding cap is applied inside BuildEmbeddingInputFromValues, not at its call sites, because that string is both what is sent to the provider and what the write-time supersession guard compares against.

… [ENG-3908]

Follow-up to the cap itself, from the review pass: - openapi.yaml, search_handler.go and models/embeddings.go all described a search result's value_text as "the text that was embedded". After the cap that is false for exactly the records this ticket exists for, and it is the published contract (it generates the API reference and the SDK docs), so the description now carries the caveat the docs-site note already had. - failed_terminal in the OpenAPI schema, and the reconciliation section of the env-var reference, described a terminal failure only as something a provider refused. A translation over TRANSLATION_MAX_INPUT_BYTES is refused before any provider call, so retrying it costs nothing — and, more usefully, raising the variable later does NOT clear the markers already written; the same correction applies to .env.example and the config comment, which had implied a tenant could simply retry after an edit. - The data-model note promised that raising EMBEDDING_MAX_INPUT_BYTES "embeds more of it". It does not: the backfill fills missing rows and never overwrites an existing vector, so already-embedded records keep the short vector until their rows are deleted and rebuilt. The note now names the lever that exists. Also: the trimmed counter's description says it counts once per attempt (a retry of a trimmed call counts again); the builder comment no longer claims a cap always leaves room for the label ahead of the answer, which is false for a large multi-byte label at a small cap.

… failure [ENG-3908] The translation refusal runs before any provider call, but the shared worker's classify-error path counted it as `translation_api_failed` and logged "provider failed permanently" at Error. That pages whoever alerts on provider errors for a call that never happened, and leaves the skip indistinguishable from a provider's own length rejection, which shares the `hub_enrichment_terminal_total{reason="length"}` series. The refusal now wraps a package sentinel (`errInputOverCap`) that the shared worker recognises: it counts under `hub_translation_worker_errors_total {reason="input_over_cap"}` (the per-enrichment skip counter the ticket asks for) and logs a Warn that says no call was made. Terminal handling, the durable `length` marker, and JobCancel are unchanged. Also corrects two comments: the search-result model claimed a vector covers "at least" the first N bytes (it is at most), and the cap floor's rationale said a sub-rune cap could only cut to nothing (true only when the text starts with a longer rune). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-14T15:26:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: de0dfed2-56c7-49cb-8d88-4ef4a9a99841

📥 Commits

Reviewing files that changed from the base of the PR and between 541af90 and d71cd6a.

📒 Files selected for processing: 41

.env.example
AGENTS.md
cmd/api/app.go
cmd/backfill-classify/main.go
cmd/backfill-embeddings/main.go
cmd/backfill-translations/main.go
cmd/worker/app.go
docs/src/content/docs/core-concepts/data-model.mdx
docs/src/content/docs/core-concepts/translated-feedback.mdx
docs/src/content/docs/reference/environment-variables.mdx
docs/src/content/docs/reference/metrics.mdx
internal/api/handlers/search_handler.go
internal/config/config.go
internal/config/config_test.go
internal/models/embeddings.go
internal/models/enrichment_failure.go
internal/observability/aggregate.go
internal/observability/enrichment_trimmed.go
internal/observability/enrichment_trimmed_test.go
internal/observability/names.go
internal/service/embedding_provider.go
internal/service/embedding_provider_test.go
internal/service/input_cap.go
internal/service/input_cap_test.go
internal/workers/enrichment_worker.go
internal/workers/feedback_embedding.go
internal/workers/feedback_embedding_tei_test.go
internal/workers/feedback_embedding_test.go
internal/workers/feedback_emotions.go
internal/workers/feedback_emotions_test.go
internal/workers/feedback_sentiment.go
internal/workers/feedback_sentiment_test.go
internal/workers/feedback_translation.go
internal/workers/feedback_translation_test.go
internal/workers/input_limit.go
internal/workers/input_limit_test.go
internal/workers/wiring.go
openapi.yaml
tests/feedback_emotions_test.go
tests/feedback_sentiment_test.go
tests/feedback_translation_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The change adds configurable UTF-8 byte caps for embedding, translation, sentiment, and emotions inputs. Embeddings, sentiment, and emotions trim input to the configured limit and record trimming metrics. Translation refuses over-cap input before a provider call and records a terminal failure. Worker wiring, configuration validation, tests, API descriptions, and documentation are updated to reflect these behaviors.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d71cd

The change caps provider input sizes with documented defaults and metrics, and the supplied evidence shows no blocking risk. It is ready to merge after the usual checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 44.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 104 functions across 34 files. (7 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title uses the Conventional Commits format and clearly summarizes the primary change: limiting text sent to enrichment providers.
Description check		The description is complete and directly addresses the template. It explains the change, issue, behavior, testing, API and documentation updates, and checklist status. The optional coverage command wa…

Full details: Description check

Explanation

The description is complete and directly addresses the template. It explains the change, issue, behavior, testing, API and documentation updates, and checklist status. The optional coverage command was not run, but this does not prevent a pass.

Full details: Docstring Coverage

Explanation

Docstring coverage is 44.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 104 functions across 34 files. (7 skipped: 7 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Fix all pre-merge checks with AI

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

pandeymangg

1 major — reviewed at d71cd6ac0d

Checked

go test on service, workers, config, observability — pass
Every provider call site goes through the cap (search query excepted)
TextCap.Apply never splits a rune or indexes out of range
Over-cap refusal reaches JobCancel, length marker, failed_terminal
Integration tests not run locally (no Postgres); CI green on pg16–18

Findings

🟠 Major · correctness internal/workers/input_limit.go:22 — input_over_cap is exported as reason="other", so the documented series never appears

Reply per finding: the fix, why it's wrong, or a ticket.

Opus 5.5

…other [ENG-3908] The translation worker-errors adapter passes every reason through allowedTranslationWorkerReasons and buckets anything unlisted as "other". d71cd6a recorded the over-cap skip as "input_over_cap" without adding it to that allowlist, so the documented series never appeared: the real exporter emitted reason="other". The unit test missed it because the counting double does not apply the allowlist. The reason is now a constant beside the allowlists (observability.WorkerReasonInputOverCap), the worker uses it, and the translation allowlist names it. A new worker test runs the refusal through the real NewTranslationMetrics and asserts the exported series is exactly {input_over_cap: 1}; it fails with {other: 1} before the allowlist change. AGENTS.md's input-cap recipe now says a refusing pipeline must extend its allowlist and assert through the real metrics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

xernobyl and others added 4 commits October 13, 2026 14:45

docs: state the enrichment input-cap invariant in the framework guide…

c32d168

… [ENG-3908]

xernobyl marked this pull request as ready for review October 14, 2026 15:20

xernobyl requested a review from pandeymangg October 14, 2026 15:20

xernobyl enabled auto-merge October 15, 2026 11:55

pandeymangg reviewed Oct 16, 2026

View reviewed changes

Comment thread internal/workers/input_limit.go Outdated

xernobyl requested a review from pandeymangg October 16, 2026 10:19

pandeymangg approved these changes Oct 16, 2026

View reviewed changes

xernobyl added this pull request to the merge queue Oct 16, 2026

Merged via the queue into main with commit 3a4efbb Oct 16, 2026
11 checks passed

xernobyl deleted the feat/ENG-3908_cap-enrichment-input branch October 16, 2026 10:35

Navigation Menu

Repository navigation

feat: cap the text sent to enrichment providers [ENG-3908] - #151

feat: cap the text sent to enrichment providers [ENG-3908]#151
xernobyl merged 5 commits into
mainfrom
feat/ENG-3908_cap-enrichment-input

xernobyl commented Oct 13, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading

Uh oh!

pandeymangg left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

xernobyl commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Decisions worth a reviewer's attention

Not in this PR

Review outcomes (this round)

Review outcomes (second round)

Review outcomes (Anshuman's review)

How should this be tested?

Checklist

Required

Appreciated

Uh oh!

coderabbitai Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

pandeymangg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xernobyl commented Oct 13, 2026 •

edited

Loading

coderabbitai Bot commented Oct 14, 2026 •

edited

Loading