feat: add filters and sort control to the feedback-record list (ENG-2059) - #120
Open
xernobyl wants to merge 12 commits into
Open
feat: add filters and sort control to the feedback-record list (ENG-2059)#120xernobyl wants to merge 12 commits into
xernobyl wants to merge 12 commits into
Conversation
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.
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.
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.
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.
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.
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.
…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.
…ct (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.
…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.
…NG-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.
✱ Stainless preview buildsThis PR will update the Edit this comment to update it. It will appear in the SDK's changelogs. ✅ hub-typescript studio · code · diff
✅ hub-openapi studio · code · diff
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
xernobyl
marked this pull request as ready for review
August 7, 2026 08:57
xernobyl
enabled auto-merge
August 7, 2026 08:57
WalkthroughFeedback-record endpoints now support repeatable identity and enum filters, timestamp and numeric ranges, enrichment filters, configurable sorting, and expanded validation. Repository queries use tenant guards, dynamic ordering, keyset predicates, and new indexes. Cursors carry sort metadata while retaining legacy compatibility. API responses distinguish malformed cursors from ordering mismatches. OpenAPI schemas and unit, repository, service, and integration tests were updated. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@internal/models/feedback_records_filters.go`:
- Line 80: Update the SubmissionID validation tag in the feedback filters model
to cap each element at 255 characters while retaining the existing count and
no-null-byte validation. Also update the OpenAPI FeedbackRecordsSubmissionId
items schema to use the same max length.
- Around line 157-169: Update ListFeedbackRecordsFilters.InvertedRanges to
initialize inverted with capacity for all five appended range pairs, and change
the capacity-hint comment to say “the five pairs below.”
In `@internal/service/feedback_records_service.go`:
- Around line 340-352: Update the cursor validation in the feedback-record
listing flow around DecodeKey and Key.Match so metadata-free legacy cursors are
accepted only for the legacy default ordering (collected_at DESC) and rejected
for other sort/order combinations, including created_at ASC. Add a regression
test covering a legacy cursor with sort=created_at and order=asc, preserving
acceptance for the default ordering.
In `@openapi.yaml`:
- Line 195: Update the type URI in both the validation example and the
cursor_sort_mismatch example to use the document-wide validation problem URI,
https://hub.formbricks.com/problems/validation, matching the existing validation
examples.
In `@tests/feedback_record_filters_test.go`:
- Around line 296-326: Add a translated record to the presence-partition test
setup and extend the table with a has_translation case that sets
ListFeedbackRecordsFilters.HasTranslation and expects translated versus
untranslated IDs. Correct the wantoutID field name and update the assertions and
partition expectations to use it consistently.
🪄 Autofix
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: bc2deaff-fc9e-495e-810a-341ea9f0ebb9
📒 Files selected for processing (22)
.gitignoreinternal/api/response/errors.gointernal/api/response/response_test.gointernal/api/validation/validation.gointernal/api/validation/validation_test.gointernal/models/feedback_records.gointernal/models/feedback_records_filters.gointernal/models/feedback_records_filters_test.gointernal/repository/feedback_records_repository.gointernal/repository/feedback_records_repository_test.gointernal/repository/filter_conditions.gointernal/repository/filter_conditions_test.gointernal/repository/list_ordering.gointernal/repository/list_ordering_test.gointernal/service/feedback_records_service.gointernal/service/feedback_records_service_test.gomigrations/021_feedback_record_filter_indexes.sqlopenapi.yamlpkg/cursor/cursor.gopkg/cursor/cursor_test.gotests/feedback_record_filters_test.gotests/feedback_value_id_test.go
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.
…NG-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.
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
1 participant
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?
Makes the feedback-record list usable at scale.
GET /v1/feedback-recordsand/countgain 15 newfilters plus sort control. Both endpoints share one filter struct, so every filter applies
identically to both and a count always describes the list it is counting.
Hub half of ENG-2059. The Formbricks gateway and MCP
wiring is a follow-up PR — it cannot land until this ships in a Hub release and the
@formbricks/hubSDK regenerates from the spec. ENG-2060builds the filter UI on top of this.
Filter semantics: AND across filters, OR within a filter
This is the part to be explicit about, because it is the whole ergonomic contract:
?source_type=survey&source_type=review?source_type=survey&has_sentiment=trueSo
?source_type=survey&source_type=review&sentiment=negativereads as"(survey OR review) AND negative" — a conjunction of disjunctions, which is exactly the shape a
filter bar produces. Each repeatable filter becomes one
col = ANY($n);emotionsuses arrayoverlap
&&, which is also ANY.What this deliberately cannot express is OR across different fields — "negative sentiment OR
rating ≤ 2". That belongs to saved segments rather than a filter bar. If it is ever needed, the
shape is
POST /v1/feedback-records/querytaking a JSON tree, compiled through the same core solist/count/query cannot drift. Not built here.
Filters
submission_idsource_typesource_idfield_idfield_group_idfield_typevalue_iduser_idsource_namelanguagecreated_since/created_untilvalue_number_min/_maxvalue_date_min/_maxsentimentemotionssentiment_score_min/_maxhas_sentimenthas_emotionshas_translationsort=collected_at|created_at,order=asc|descA few decisions worth knowing:
col = ANY($1)over a one-element array is thesame
ScalarArrayOpExprthe planner produced forcol = $1; a test pins the legacy placeholderlayout so this stays true.
created_atis notcollected_at. They diverge on a historical re-import — oldcollected_at, today'screated_at— so "what did this import bring in" is acreated_atquestion and needs its own pair.
emotionsmatches ANY, not ALL. Two selected chips read as "anger or fear".field_typebinds through::text[]::field_type_enum[]. The column is a PostgreSQL ENUM andpgx has no codec for
field_type_enum[]; without the cast it works only via a reflection fallbackthat resolves
[]stringto_textby coincidence.value_number_min=0selects onlyrecords that carry a number at all;
sentiment_score_min=-1means "all enriched records". Eachparameter description says so.
updated_atis deliberately not a sort key. The enrichment workers and every PATCH bump it, soa row can move across the cursor between pages and be silently dropped. A change feed needs an
append-only sequence, not a sort parameter.
since/untilpair, which previously returned an empty page when supplied backwards. Aninverted range can only ever match zero rows, so it is a client bug, and a 400 naming the pair
beats a page the caller reads as "there is no such feedback".
buildFilterConditionsnow fails closed on a missing tenant. It previously emitted no tenantpredicate at all when
tenant_idwas nil — a query spanning every tenant, prevented only by theHTTP layer's
requiredtag, which workers and tests bypass. Reachable in practice: awhitespace-only
tenant_idsatisfiesrequiredand is caught by this guard.Cursors
The cursor now records the ordering it was issued under. Presenting it with a different
sortororderreturns 400 rather than a page that looks like a continuation and is not (arbitrary rowsskipped, others repeated, no way for the client to notice).
Cursors already in client hands keep working on the default listing: the new
s/ofields areomitemptyand declared aftert/i, so a cursor with no recorded ordering is byte-identicalto the old format. A golden test pins that wire format — the package is shared with the webhooks
endpoints, which are untouched.
Such a cursor is not treated as a wildcard. It was issued before sort control, so it holds a
position in the only ordering that existed then (
collected_at desc); it is resolved to exactlythat and refused anywhere else. Accepting it under
sort=created_atwould bind a collectedtimestamp to the
created_atkeyset predicate and skip or repeat rows — the same defect the guardexists to prevent. (Caught in review; see 9d2e77d.)
Indexes (migration 021)
Three, all
CONCURRENTLY:(tenant_id, created_at DESC, id)created_atfilter had zero coverage; the shape mirrors 006 so it also servesORDER BY created_at DESC, id ASCwith no sort step(tenant_id, value_date) WHERE NOT NULLdatefields populate it(tenant_id, source_name) WHERE NOT NULLsource_idhas had an index since 001;source_namenever didVerified with
EXPLAINon 60k rows:created_at DESCis a pure index-only scan, andcreated_at ASCis an index-only scan backward plus an incremental sort (presorted keycreated_at) — sothe mirrored ASC index is deliberately not created.
feedback_recordsalready carries ~22 indexes;the migration documents what is left unindexed (
language,value_number, the FALSE form of thepresence filters) and why.
Not in this PR
Free-text search (needs
pg_trgm, a superuser-gated extension — semantic search covers part of theneed today, though it takes no filters), a facets endpoint for ENG-2060's dropdowns, bulk delete
(ENG-2129), and metadata/JSONB filtering.
Resource bound
The
max=caps are the only bound on a repeated query parameter — go-playground/form imposes noneof its own. The remaining exposure is the URL itself, bounded by net/http's
MaxHeaderBytes(1 MBdefault, not overridden here), since values are materialized while decoding and rejected afterwards.
How should this be tested?
Config:
DATABASE_URLpointing at atest_dbwith migrations applied,API_KEYfrom.env.Automated
New-code coverage is 98.1% (252/257 added statements); every file but
validation.gois at 100%,and its remainder is unreachable defensive branches matching the pre-existing validators beside them.
Integration tests worth looking at (
tests/feedback_record_filters_test.go):field_typemulti-value against a real DB — settles the enum cast, which no string assertion canemotionsreturns a{joy}record and a{joy,anger}record for?emotions=joy&emotions=angercollected_at+ newcreated_atis matched bycreated_sinceand notsince(sort, order)combinations traverse 25 records with deliberatecollected_atties inpages 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.
Manual smoke test — 52/52
Four records in a throwaway tenant (
q1text/enriched,q2rating/enriched,q3nps,q4date):since/until/created_*)has_*)/countparity checkstenant_idRepresentative 400 bodies:
Review round
Five review comments addressed in 9d2e77d, one of them a real defect (the legacy-cursor hole above).
Smoke suite re-run afterwards: still 52/52. The behaviours the fixes changed were then checked over
HTTP, since the suite does not cover them:
sort=created_atorder=ascsort=collected_at&order=descsubmission_id255 / 256 charsSecurity
A security review found one real defect, fixed here: go-playground/form consults its custom type
funcs only for the plain query key, so the indexed form (
?field_type[0]=bogus) skipped them andassigned raw strings. Values were always bound as parameters — never SQL injection, never a tenant
crossing — but
field_typethen failed the enum cast at Postgres as an unmapped 500, andsentiment/emotionswere accepted silently. Fixed withdiveelement validators, which run onthe decoded struct however it was populated; regression-tested.
Also verified: all identifiers in generated SQL come from typed constants (
sqlColumn/sqlOperator)with an exhaustive fail-closed allowlist for
sort;gitleaksandsemgrep(p/golang,p/sql-injection) clean on every changed file;govulncheck0.Checklist
Required
make buildmake tests(integration tests intests/)make fmtandmake lint; no new warningsgit pull origin mainmigrations/with goose annotations and ranmake migrate-validateAppreciated
make testsor API contract workflow)docs/if changes were necessary — nodocs/changes needed; the OpenAPI spec is the referencemake tests-coveragefor meaningful logic changesDeploy order: migration first, then code. Serving
sort=created_atbeforeCREATE INDEX CONCURRENTLYfinishes is exactly the residual scan this removes.