Skip to content

Tags: formbricks/hub

Tags

0.8.4

Toggle 0.8.4's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
feat: add filters and sort control to the feedback-record list (ENG-2…

…059) (#120)

* feat: add enum filter parsing and sort field types (ENG-2059)

Groundwork for the repeatable enum filters and sort control on the
feedback-record list endpoint. No behavior change yet — nothing calls
these.

- ParseFieldTypes / ParseSentimentValues / ParseEmotionValues parse the
  repeated occurrences of one query parameter into deduplicated labels.
  They live here rather than behind a `dive` validate tag so that
  `?field_type=` keeps meaning "no filter" (a dive tag would turn it
  into a 400), and so a rejected label is attributed to the plain
  parameter name instead of `sentiment[2]`.
- InvalidSentimentValueError / InvalidEmotionValueError mirror the
  existing InvalidFieldTypeError, carrying the rejected value and
  unwrapping to a sentinel.
- SortField / SortOrder allowlist the two columns that are safe to
  keyset on. updated_at is deliberately excluded: the enrichment
  workers and every PATCH bump it, so a row can move across the cursor
  between pages and be silently dropped.

* feat: make list cursors sort-aware (ENG-2059)

A keyset cursor's timestamp bounds one specific column in one specific
direction. Once the list endpoint accepts a sort parameter, presenting
a cursor under a different ordering still produces a valid,
index-served query and a plausible-looking page — just not the
continuation of the previous one, with an arbitrary set of rows
skipped and another repeated, and no way for the client to notice.

Record the ordering in the cursor and refuse a mismatch:

- Key/EncodeKey/DecodeKey/Match carry the (sort, order) the cursor was
  issued under. Encode/Decode stay as wrappers so webhooks, which
  share this package and have one fixed ordering, keep a single code
  path.
- The payload's s/o fields are omitempty and declared after t/i, so a
  cursor with no recorded ordering is byte-identical to the old
  format. Cursors already in client hands keep working, and a legacy
  cursor matches any ordering.
- ErrCursorSortMismatch is a standalone sentinel, deliberately not
  wrapping ErrInvalidCursor, so the response layer can tell a client
  to keep its sort unchanged rather than to start over.

The payload carries a position and nothing else; the doc comment says
why tenancy must never be added to it.

* feat: decode repeatable enum query filters (ENG-2059)

Register form-decoder type funcs for []FieldType, []SentimentValue and
[]EmotionValue so a repeated query parameter (?sentiment=negative
&sentiment=very_negative) decodes into a validated, deduplicated slice.

The decoder resolves a custom type func from the field's own type
before it reaches its slice branch, and a top-level struct field is
decoded at index 0, so each func receives every repetition at once.
Without a registration the elements would decode through the plain
string branch and skip enum validation entirely.

Parsing here rather than with a `dive` validate tag keeps ?field_type=
meaning "no filter" — a dive tag rejects the empty string — and
attributes a bad label to the plain parameter name instead of
"sentiment[2]", which a client cannot act on.

Nothing uses these yet; the filter struct still holds scalars.

* feat: add feedback-record list filters and sort control (ENG-2059)

Make the feedback-record list usable at scale. GET /v1/feedback-records
and /count share one filter struct, so both gain all of this at once.

Filters:
- the eight identity filters become repeatable, emitted as
  `col = ANY($n)`. A single occurrence produces the same plan and the
  same rows as the scalar equality did, so this is backward
  compatible; a test pins the legacy placeholder layout.
- new: source_name, language, created_since/created_until (created_at
  diverges from collected_at on a historical re-import), value_number
  and value_date ranges, sentiment, emotions, sentiment_score range,
  and has_sentiment/has_emotions/has_translation.
- emotions matches with `&&` (ANY), not `@>` (ALL): two selected chips
  read as "anger or fear". `&&` is strict, so the partial GIN index
  still applies.
- field_type binds through `::text[]::field_type_enum[]`. The column
  is a PostgreSQL ENUM and pgx has no codec for field_type_enum[];
  without the cast it works only via a reflection fallback that
  resolves []string to _text by coincidence.
- an inverted min/max pair is a 400 naming the lower bound rather than
  an empty page the caller reads as "no such feedback". This now also
  covers the pre-existing since/until pair, which previously returned
  an empty 200 when supplied backwards.

Sort:
- sort=collected_at|created_at and order=asc|desc, defaulting to the
  ordering the endpoint already had. resolveListOrdering is an
  exhaustive allowlist that fails closed rather than interpolating a
  request value, and it is tested by driving it directly from Go the
  way a worker would, bypassing HTTP validation.

buildFilterConditions now fails closed when tenant_id is missing or
blank. It previously emitted no tenant predicate at all in that case —
a query spanning every tenant, prevented only by the HTTP layer's
`required` tag, which workers and tests bypass. The condition also
moved into a grouped helper as part of this refactor, which is exactly
the kind of move that loses a guard silently, so there is a direct
unit test for it.

* feat: wire sort through the feedback-record list service (ENG-2059)

Resolve sort and order to their defaults once in the service, so the
keyset predicate, the ORDER BY and the next cursor are all derived
from the same values and cannot disagree about what a page is ordered
by.

The next cursor now carries the last row's value from whichever column
the listing is sorted by, plus the ordering itself, and an incoming
cursor is checked against the request before it reaches the
repository. A cursor issued under a different ordering is refused
rather than served a page that looks like a continuation and is not.
A cursor with no recorded ordering — anything issued before this
change — still works.

Also guards the records[len-1] index behind the pagination invariant,
mirroring the webhooks list path. It is unreachable today because
hasMore implies a non-empty page, but an out-of-range index would
panic in the request goroutine instead of surfacing as an error.

The repository mock's List/ListAfterCursor returned "not implemented",
so the service list path had no unit coverage at all; it now returns a
configurable page and captures what the service passed in.

* feat: index the new feedback-record filters (ENG-2059)

Three indexes for the filters that had no usable coverage.

(tenant_id, created_at DESC, id) is the one that matters: filtering
created_at while ordering by collected_at cannot use the existing
keyset index for the ordering, so a narrow created_at window over a
wide collected_at range degrades to a residual scan. The DESC/id shape
mirrors 006, so this index serves both the filter and
`ORDER BY created_at DESC, id ASC` with no sort step. The ASC
direction reads it backward plus an incremental sort on id, whose tie
groups are effectively size 1 because created_at is a server NOW() per
single-row transaction — so the mirrored ASC index is not created.

value_date and source_name get partial indexes, both sparse columns,
matching the shape 014/015/018 use.

The migration documents what is deliberately left unindexed and why:
language is too low-cardinality to be chosen, value_number already has
a non-tenant-prefixed index whose replacement needs an EXPLAIN rather
than a guess, and the FALSE form of the presence filters selects
exactly the rows the existing partial indexes exclude.

* test: cover the feedback-record filters against a real database (ENG-2059)

The unit tests assert on generated SQL; these assert on rows, which is
the only place several of these decisions can actually be checked.

- field_type multi-value settles the ::text[]::field_type_enum[] cast.
  It is the one filter whose column is a PostgreSQL ENUM, and no
  amount of string assertion proves the cast is valid SQL.
- emotions returns both a {joy} record and a {joy,anger} record for
  ?emotions=joy&emotions=anger. A containment implementation returns
  one; this test is the whole specification of ANY-vs-ALL.
- every range filter includes a row sitting exactly on the bound.
- a record with an old collected_at and a new created_at — the
  historical re-import shape — is matched by created_since and not by
  since, which is what stops the two pairs being collapsed into one.
- every new filter runs against two tenants holding identical data.
- the four (sort, order) combinations each traverse 25 records with
  deliberate collected_at ties in pages of four, asserting the result
  is a permutation of the seeded set: nothing skipped, nothing
  repeated, globally ordered across page boundaries. Without the ties
  a broken tiebreak passes.
- a cursor cannot be carried to another ordering, and one issued
  before sort control existed still works.

Verified with EXPLAIN on 60k rows that created_at DESC is a pure
index-only scan on the new index and created_at ASC is an index-only
scan backward plus an incremental sort, so no mirrored ASC index is
needed.

* docs: document the feedback-record filters and sort in the API contract (ENG-2059)

The spec is hand-authored and Schemathesis generates requests from it,
so it is the contract, not a description of one.

- The eight identity filters become arrays with an explicit
  `style: form, explode: true`. That is the OpenAPI default, but it is
  also the whole agreement with the Go decoder: `explode: false` would
  render comma-separated values, which go-playground/form does not
  split. These are the spec's first array-valued query parameters.
- 15 new parameters, added to BOTH the list and count operations. The
  count endpoint documents itself as taking the same parameters as
  list, and the two share one filter struct, so a parameter on one and
  not the other would be a spec lie.
- sort and order are list-only, alongside limit and cursor. Sort
  cannot change a COUNT(*), and documenting a parameter with no effect
  invites misuse.
- FieldType, SentimentValue and EmotionValue become named schemas.
  field_type was written out inline three times and was about to gain
  a fourth; three sources of truth with the Go constants and the DB
  CHECK is already the limit.
- The cursor description now states that a cursor is bound to the
  ordering it was issued under, and the 400 carries worked examples
  for an inverted range and a cursor/sort mismatch.

Also documents the 401 both operations have always been able to
return — `security: [ApiKeyAuth]` is global, but only 8 of 30
operations said so — and the value_number examples now form a valid
range. Schemathesis caught that pair: taken together the old examples
described a request the new inverted-range check rejects.

* fix: validate repeatable enum filters on the indexed query form (ENG-2059)

Security review found the "no unknown label" invariant was false.

go-playground/form consults its custom type funcs only when the plain
query key is present. Its indexed form takes a different branch and
skips them entirely, so `?field_type[0]=bogus` decoded to
[]FieldType{"bogus"} with no error. For field_type that reached
Postgres and failed the field_type_enum cast as an unmapped 500 where
a 400 belongs; for sentiment and emotions it was accepted silently and
compared as raw text. The values were always bound as parameters, so
this was never SQL injection and never crossed a tenant boundary — but
the invariant that addAnyOfEnum relies on did not hold, and the gap
was untested.

Add dive element validators as the gate that actually holds it: they
run on the decoded struct however it was populated. The decode-time
parsing stays, because it is what keeps `?field_type=` meaning "no
filter" and what attributes a bad label to the bare parameter name
rather than to sentiment[2]. The max caps now also bound the indexed
path, where the decoder's dedupe does not run.

Also from the review pass:

- Untrack .schemathesis/, swept in by a `git add -A`, and gitignore
  it. Local run cache; no credentials (Authorization was filtered).
- Split ordering out of filter_conditions.go into list_ordering.go,
  and the query-side model types out of feedback_records.go into
  feedback_records_filters.go. Ordering is not a filter concern, and
  the model file was carrying the record, its requests, and the whole
  list-query surface at 829 lines.
- Drop the dead WHERE branch in ListAfterCursor. buildFilterConditions
  fails closed without a tenant, so the clause it returns is never
  empty and the joiner is always " AND " — pinned by a test rather
  than left as an unreachable branch.

Coverage on new code is 98.1% (252/257 statements). The five
uncovered are unreachable defensive branches — RegisterValidation
failure paths and reflect.Kind guards — matching the shape of the
pre-existing validators beside them.

* test: pin the repeatable-filter value caps to the sets they mirror (ENG-2059)

Re-review finding. A struct tag cannot reference a constant, so the
`max=` caps added with the dive validators were nine hand-written
literals with nothing tying them to anything.

The enum caps are the ones that matter. `max=6` on emotions mirrors
the Ekman six; adding a seventh label — a live possibility — would
leave the cap in place and start rejecting a request that legitimately
asks for all seven, with no test going red. TestFilterValueCapsMatch
TheirSets reflects over the tags and asserts each enum filter caps at
its own label-set cardinality and each string filter at
MaxFilterValues. Verified it fails on a drifted cap, not just on a
correct one.

MaxFilterValues was also left describing a rule it no longer stated
accurately, since the enum filters do not use it; its comment now says
which filters it governs and points at the test that enforces it.

* fix: refuse a legacy cursor under a non-default ordering (ENG-2059)

Review found a hole in the guard this branch exists to add.

Key.Match treated a cursor with no recorded ordering as a wildcard, so
it matched any sort. But such a cursor is not orderless — it was
issued before sort control, which means it holds a position in the one
ordering the endpoint had then, collected_at DESC. Presenting it with
sort=created_at bound that collected timestamp to the created_at
keyset predicate, skipping or repeating records: precisely the failure
the cursor rework was written to prevent.

Match now compares exactly, and the legacy rule moves to the caller as
Key.ResolveOrdering(defaultSort, defaultOrder). That keeps pkg/cursor
free of endpoint policy — it is shared with webhooks — and puts the
knowledge of what the original ordering was in the service that owns
it. Old cursors keep working on the default listing and are refused
anywhere else. Covered at the cursor, service and integration levels,
and the OpenAPI cursor description no longer claims they "remain
valid" unqualified.

Also from the review:

- submission_id caps element length at 255 like every other repeatable
  string filter. It is stored as VARCHAR(255), so a longer value could
  never match a row and only widened the bound array. OpenAPI items
  schema updated to match.
- InvertedRanges pre-allocates for five pairs, not four.
- The two new problem examples used a type URI that appears nowhere
  else; every other validation example in the spec uses
  hub.formbricks.com/problems/validation, which is also what the
  server actually emits.
- has_translation is now covered in the presence-partition test, with
  the fixture reworked so it discriminates: a record carrying
  sentiment and emotions but no translation means a predicate reading
  the wrong column now fails. Verified by pointing it at the sentiment
  column and watching the test go red. Also fixes the wantoutID typo.

* docs: correct the Key doc and strengthen the presence-filter tests (ENG-2059)

Follow-up re-review of 9d2e77d.

The Key type doc still said "Empty means unspecified: Match accepts it
against any ordering" — the exact behaviour 9d2e77d removed. Match's
own doc was rewritten and ResolveOrdering documented, but the type doc
three declarations above was missed, so it now described the bug as
if it were the contract. Someone trusting it could reintroduce the
wildcard.

The presence-partition fixture also overclaimed. Three records left
has_sentiment and has_emotions returning identical sets, so those two
stayed silently swappable and only has_translation was really pinned.
A fourth record (sentiment, no emotions) gives all three filters a
distinct partition. Verified by pointing each of the three at a wrong
column in turn and confirming each is now detected — previously only
one of the three was.

TestFilterValueCapsMatchTheirSets read only the slice-length half of
each validate tag, so the per-element caps it appeared to cover were
unpinned — including the max=255 just added to submission_id.
TestStringFilterElementCapsMatchTheStoredWidth now pins each to its
column width, so a cap set below the stored width (which would reject
a legitimate value) fails instead of shipping. Verified against a
drifted cap.

* fix: address ENG-2059 PR review findings (Dhru)

Addresses the 4 open review comments on #120.

- migrations/021: sentiment_score is a range filter with no index of
  its own and was missing from both the index set and the "deliberately
  unindexed" note. Documented the omission instead of adding an index:
  the feedback_records_sentiment_pairing CHECK from 014 makes sentiment
  and sentiment_score NULL together, so the existing partial
  (tenant_id, sentiment) index already narrows to the candidate rows
  whenever the score filter is combined with a sentiment filter — only
  the uncombined case is exposed, and a permanent write-path cost isn't
  worth paying for that on a guess. Left the ready-to-run index for
  whoever does the EXPLAIN.

- feedback_records_filters_openapi_test.go: new test tying openapi.yaml's
  maxItems/items.maxLength back to the Go-side caps they're supposed to
  mirror, extending the existing reflection-based
  TestFilterValueCapsMatchTheirSets as suggested. Nothing previously
  caught a spec that understates what the API accepts — schemathesis
  fuzzes inside the documented bound, so a stale maxItems would pass
  silently. Mutation-tested five ways (understated maxItems, understated
  maxLength, removed maxItems, an undocumented string filter, and a
  differently-sized element cap on language) to make sure it actually
  fails on drift rather than just running green.

- feedback_records_filters_test.go: corrected
  TestStringFilterElementCapsMatchTheStoredWidth's comment. source_type
  and source_name are bare unbounded VARCHAR (confirmed against the live
  DB), not VARCHAR(255) — for those two the 255 cap is an API-level
  choice, not a mirror of the schema, so a value between 256 and
  unbounded is now storable but not filterable.

- openapi.yaml: sentiment and emotions descriptions now state that an
  empty value is a no-op, matching field_type's existing wording and the
  actual parseEnumValues behavior all three share.

Smoke-tested against a live hub-api + Postgres: confirmed the served
/openapi.yaml reflects the new descriptions, empty sentiment=/emotions=
return 200 (not 400), sentiment_score_min/max both accept a valid range
and reject an out-of-range value, and the new 255-char cap 400s at 256
while still accepting exactly 255.

* fix: document /count 400 response and filters in-place mutation (ENG-2059)

The /v1/feedback-records/count 400 response now mirrors the list endpoint's
detailed description and inverted_range example, so both endpoints document
the same validation failures consistently.

ListFeedbackRecords now documents that it mutates *filters in-place to fill in
default Sort/Order/Limit — safe because the HTTP handler constructs a fresh
struct per request.

0.8.3

Toggle 0.8.3's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

0.8.2

Toggle 0.8.2's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
fix(taxonomy): recheck run freshness before reaping (#110)

0.8.1

Toggle 0.8.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

0.8.0

Toggle 0.8.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

0.7.1

Toggle 0.7.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

0.7.0

Toggle 0.7.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

0.6.1

Toggle 0.6.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
fix: pass release tag to GHCR workflow (#93)

0.6.0

Toggle 0.6.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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>

0.50

Toggle 0.50's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
chore: no tenant_id on similar search (#82)

* chore: no tenant_id on similar search

* chore: address PR comments

* chore: tighten openapi doc