Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions internal/models/taxonomy.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,14 @@ const (
)

// TaxonomyScope identifies the feedback field that a taxonomy run covers.
//
// SourceID is optional: feedback_records.source_id is nullable, so feedback may have no
// attributed source. An empty SourceID is the canonical "no source" bucket and matches
// feedback records whose source_id is NULL or blank.
type TaxonomyScope struct {
TenantID string `json:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"`
SourceType string `json:"source_type" validate:"required,no_null_bytes,min=1,max=255"`
SourceID string `json:"source_id" validate:"required,no_null_bytes,min=1,max=255"`
SourceID string `json:"source_id" validate:"omitempty,no_null_bytes,max=255"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
FieldID string `json:"field_id" validate:"required,no_null_bytes,min=1,max=255"`
}

Expand Down Expand Up @@ -116,12 +120,17 @@ type CreateTaxonomyRunResponse struct {
}

// ListTaxonomyRunsFilters scopes taxonomy run history queries.
//
// SourceID is a tri-state filter: nil means "no source_id filter", while a non-nil
// pointer (including a pointer to "") filters by that exact value. An empty string
// scopes history to the canonical "no source" bucket — runs whose source_id is "" —
// which a plain string filter cannot express (it cannot tell "unset" from "empty").
type ListTaxonomyRunsFilters struct {
TenantID string `form:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"`
SourceType string `form:"source_type" validate:"omitempty,no_null_bytes,min=1,max=255"`
SourceID string `form:"source_id" validate:"omitempty,no_null_bytes,min=1,max=255"`
FieldID string `form:"field_id" validate:"omitempty,no_null_bytes,min=1,max=255"`
Limit int `form:"limit" validate:"omitempty,min=1,max=100"`
TenantID string `form:"tenant_id" validate:"required,no_null_bytes,min=1,max=255"`
SourceType string `form:"source_type" validate:"omitempty,no_null_bytes,min=1,max=255"`
SourceID *string `form:"source_id" validate:"omitempty,no_null_bytes,max=255"`
FieldID string `form:"field_id" validate:"omitempty,no_null_bytes,min=1,max=255"`
Limit int `form:"limit" validate:"omitempty,min=1,max=100"`
}

// ListTaxonomyRunsResponse contains taxonomy run history.
Expand Down
26 changes: 18 additions & 8 deletions internal/repository/taxonomy_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (r *TaxonomyRepository) ListFieldOptions(
SELECT
fr.tenant_id,
fr.source_type,
fr.source_id,
COALESCE(NULLIF(btrim(fr.source_id), ''), ''),
COALESCE(MAX(fr.source_name) FILTER (WHERE fr.source_name IS NOT NULL AND btrim(fr.source_name) <> ''), ''),
fr.field_id,
COALESCE(MAX(fr.field_label) FILTER (WHERE fr.field_label IS NOT NULL AND btrim(fr.field_label) <> ''), ''),
Expand All @@ -66,12 +66,10 @@ func (r *TaxonomyRepository) ListFieldOptions(
FROM feedback_records fr
LEFT JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $2
WHERE fr.tenant_id = $1
AND fr.source_id IS NOT NULL
AND btrim(fr.source_id) <> ''
AND fr.value_text IS NOT NULL
AND btrim(fr.value_text) <> ''
GROUP BY fr.tenant_id, fr.source_type, fr.source_id, fr.field_id
ORDER BY fr.source_type, fr.source_id, fr.field_id`,
GROUP BY fr.tenant_id, fr.source_type, COALESCE(NULLIF(btrim(fr.source_id), ''), ''), fr.field_id
ORDER BY fr.source_type, COALESCE(NULLIF(btrim(fr.source_id), ''), ''), fr.field_id`,
tenantID, embeddingModel,
)
if err != nil {
Expand Down Expand Up @@ -127,7 +125,7 @@ func (r *TaxonomyRepository) CountScopeInput(
LEFT JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $5
WHERE fr.tenant_id = $1
AND fr.source_type = $2
AND fr.source_id = $3
AND NULLIF(btrim(fr.source_id), '') IS NOT DISTINCT FROM NULLIF(btrim($3), '')
AND fr.field_id = $4
AND fr.value_text IS NOT NULL
AND btrim(fr.value_text) <> ''`,
Expand Down Expand Up @@ -358,9 +356,21 @@ func (r *TaxonomyRepository) ListRuns(
query += fmt.Sprintf(" AND %s = $%d", column, len(args))
}

// addFilterPtr is the tri-state variant: nil skips the filter, while a non-nil
// pointer filters by the exact stored value, including "" for the canonical
// "no source" bucket (which addFilter would drop as an absent filter).
addFilterPtr := func(column string, value *string) {
if value == nil {
return
}

args = append(args, *value)
query += fmt.Sprintf(" AND %s = $%d", column, len(args))
}

addFilter("source_type", filters.SourceType)
addFilter("source_id", filters.SourceID)
addFilter("field_id", filters.FieldID)
addFilterPtr("source_id", filters.SourceID)

args = append(args, limit)
query += fmt.Sprintf(" ORDER BY created_at DESC, id DESC LIMIT $%d", len(args))
Expand Down Expand Up @@ -392,7 +402,7 @@ func (r *TaxonomyRepository) GetRunInput(
INNER JOIN embeddings e ON e.feedback_record_id = fr.id AND e.model = $6
WHERE fr.tenant_id = $1
AND fr.source_type = $2
AND fr.source_id = $3
AND NULLIF(btrim(fr.source_id), '') IS NOT DISTINCT FROM NULLIF(btrim($3), '')
AND fr.field_id = $4
AND fr.value_text IS NOT NULL
AND btrim(fr.value_text) <> ''
Expand Down
16 changes: 16 additions & 0 deletions internal/service/id_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,22 @@ func normalizeRequiredIdentifier(fieldName, value string) (string, error) {
return "", huberrors.NewValidationError(fieldName, fieldName+" is required and cannot be empty")
}

return normalizeIdentifierValue(fieldName, normalized)
}

// normalizeOptionalIdentifier trims an identifier and validates it when present.
// An empty (or blank) value is allowed and normalizes to "", which callers treat as the
// canonical "no value" bucket (e.g. taxonomy's "no source" scope).
func normalizeOptionalIdentifier(fieldName, value string) (string, error) {
normalized := strings.TrimSpace(value)
if normalized == "" {
return "", nil
}

return normalizeIdentifierValue(fieldName, normalized)
}

func normalizeIdentifierValue(fieldName, normalized string) (string, error) {
if strings.ContainsRune(normalized, '\x00') {
return "", huberrors.NewValidationError(fieldName, fieldName+" must not contain NULL bytes")
}
Expand Down
3 changes: 2 additions & 1 deletion internal/service/taxonomy_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,8 @@ func normalizeTaxonomyScope(scope models.TaxonomyScope) (models.TaxonomyScope, e
return models.TaxonomyScope{}, err
}

sourceID, err := normalizeRequiredIdentifier("source_id", scope.SourceID)
// source_id is optional: an empty value is the canonical "no source" scope.
sourceID, err := normalizeOptionalIdentifier("source_id", scope.SourceID)
if err != nil {
return models.TaxonomyScope{}, err
}
Expand Down
16 changes: 13 additions & 3 deletions migrations/010_taxonomy_schema.sql
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
-- +goose up
-- Taxonomy generation artifacts are stored as run-scoped Hub data. Keep them
-- separate from feedback_records so repeated generation never mutates source feedback.
--
-- Source scope and the "no source" bucket:
-- feedback_records.source_id is nullable (see 001_initial_schema.sql), so feedback can
-- exist with no attributed source. Taxonomy must still cover those records. Rather than
-- making source_id nullable inside the composite PK/FK/UNIQUE constraints below (NULL is
-- never equal to NULL, which breaks key matching), taxonomy canonicalizes "no source" to
-- the empty string ''. The Go layer always sends a string, never NULL, so source_id stays
-- NOT NULL here and '' is a valid, comparable key value. Repository queries that read
-- feedback_records use null-safe matching (NULLIF(btrim(source_id), '') IS NOT DISTINCT
-- FROM ...) so the '' taxonomy scope matches NULL/blank feedback rows.
ALTER TABLE feedback_records
ADD CONSTRAINT feedback_records_id_tenant_unique
UNIQUE USING INDEX feedback_records_id_tenant_uidx;
Expand Down Expand Up @@ -45,7 +55,7 @@ CREATE TABLE taxonomy_runs (
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
CONSTRAINT taxonomy_runs_tenant_id_required CHECK (btrim(tenant_id) <> ''),
CONSTRAINT taxonomy_runs_source_type_required CHECK (btrim(source_type) <> ''),
CONSTRAINT taxonomy_runs_source_id_required CHECK (btrim(source_id) <> ''),
-- source_id intentionally allows '' (the canonical "no source" bucket); see header note.
CONSTRAINT taxonomy_runs_field_id_required CHECK (btrim(field_id) <> ''),
CONSTRAINT taxonomy_runs_record_count_nonnegative CHECK (record_count >= 0),
CONSTRAINT taxonomy_runs_embedding_count_nonnegative CHECK (embedding_count >= 0),
Expand Down Expand Up @@ -157,7 +167,7 @@ CREATE TABLE taxonomy_active_runs (
ON DELETE CASCADE,
CONSTRAINT taxonomy_active_runs_tenant_id_required CHECK (btrim(tenant_id) <> ''),
CONSTRAINT taxonomy_active_runs_source_type_required CHECK (btrim(source_type) <> ''),
CONSTRAINT taxonomy_active_runs_source_id_required CHECK (btrim(source_id) <> ''),
-- source_id intentionally allows '' (the canonical "no source" bucket); see header note.
CONSTRAINT taxonomy_active_runs_field_id_required CHECK (btrim(field_id) <> ''),
UNIQUE (run_id)
);
Expand Down Expand Up @@ -185,7 +195,7 @@ CREATE TABLE taxonomy_node_events (
FOREIGN KEY (node_id, run_id) REFERENCES taxonomy_nodes(id, run_id) ON DELETE CASCADE,
CONSTRAINT taxonomy_node_events_tenant_id_required CHECK (btrim(tenant_id) <> ''),
CONSTRAINT taxonomy_node_events_source_type_required CHECK (btrim(source_type) <> ''),
CONSTRAINT taxonomy_node_events_source_id_required CHECK (btrim(source_id) <> ''),
-- source_id intentionally allows '' (the canonical "no source" bucket); see header note.
CONSTRAINT taxonomy_node_events_field_id_required CHECK (btrim(field_id) <> ''),
CONSTRAINT taxonomy_node_events_actor_id_required CHECK (btrim(actor_id) <> '')
);
Expand Down
168 changes: 168 additions & 0 deletions tests/taxonomy_no_source_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package tests

import (
"context"
"os"
"testing"

"github.com/google/uuid"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/stretchr/testify/require"

"github.com/formbricks/hub/internal/config"
"github.com/formbricks/hub/internal/models"
"github.com/formbricks/hub/internal/repository"
"github.com/formbricks/hub/pkg/database"
)

func setupTaxonomyNoSourceTestDB(t *testing.T) *pgxpool.Pool {
t.Helper()

databaseURL := os.Getenv("DATABASE_URL")
if databaseURL == "" {
databaseURL = defaultTestDatabaseURL
}

t.Setenv("DATABASE_URL", databaseURL)

cfg, err := config.Load()
require.NoError(t, err)

db, err := database.NewPostgresPool(context.Background(), cfg.Database.URL,
database.WithPoolConfig(cfg.Database.PoolConfig()),
)
require.NoError(t, err)

t.Cleanup(db.Close)

return db
}

// TestTaxonomyNoSourceScope verifies that feedback records with a NULL/blank source_id
// participate in taxonomy through the canonical "no source" bucket (empty-string scope).
func TestTaxonomyNoSourceScope(t *testing.T) {
ctx := context.Background()
db := setupTaxonomyNoSourceTestDB(t)
repo := repository.NewTaxonomyRepository(db)

tenantID := "taxonomy-nosource-" + uuid.NewString()
sourceType := "formbricks"
fieldID := "feedback"

const embeddingModel = "text-embedding-test"

t.Cleanup(func() {
_, _ = db.Exec(ctx, `DELETE FROM taxonomy_runs WHERE tenant_id = $1`, tenantID)
_, _ = db.Exec(ctx, `DELETE FROM feedback_records WHERE tenant_id = $1`, tenantID)
})

// Two feedback records with no attributed source: one NULL, one blank.
for _, srcID := range []any{nil, " "} {
_, err := db.Exec(ctx, `
INSERT INTO feedback_records (
source_type, source_id, field_id, field_label, field_type,
value_text, tenant_id, submission_id
)
VALUES ($1, $2, $3, $4, $5::field_type_enum, $6, $7, $8)`,
sourceType, srcID, fieldID, "Feedback", "text",
"Login was confusing", tenantID, "submission-"+uuid.NewString(),
)
require.NoError(t, err)
}

// Discovery surfaces the records as a single "no source" bucket with empty SourceID.
options, err := repo.ListFieldOptions(ctx, tenantID, embeddingModel)
require.NoError(t, err)

var noSource *models.TaxonomyFieldOption

for i := range options {
if options[i].SourceType == sourceType && options[i].FieldID == fieldID {
noSource = &options[i]
}
}

require.NotNil(t, noSource, "expected a discovered field option for the no-source bucket")
require.Empty(t, noSource.SourceID, "no-source bucket must expose an empty source_id")
require.Equal(t, 2, noSource.RecordCount, "NULL and blank source_id must collapse into one bucket")

// Counting the empty-source scope matches both NULL and blank feedback rows.
scope := models.TaxonomyScope{
TenantID: tenantID,
SourceType: sourceType,
SourceID: "",
FieldID: fieldID,
}

recordCount, _, _, err := repo.CountScopeInput(ctx, scope, embeddingModel)
require.NoError(t, err)
require.Equal(t, 2, recordCount, "empty-source scope must null-safe match NULL/blank source rows")

// A taxonomy run can be created for the empty-source scope and is found by the
// in-progress guard (empty string is a valid, comparable key in taxonomy tables).
run, created, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{
TaxonomyScope: scope,
RecordCount: recordCount,
EmbeddingCount: 0,
})
require.NoError(t, err)
require.True(t, created)
require.Empty(t, run.SourceID)

_, createdAgain, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{
TaxonomyScope: scope,
})
require.NoError(t, err)
require.False(t, createdAgain, "a second run for the same empty-source scope must reuse the in-progress run")
}

// TestTaxonomyListRunsSourceIDTriState verifies the tri-state source_id filter on run
// history: nil returns every source, a pointer to "" scopes to the no-source bucket, and
// a pointer to a concrete value scopes to that source.
func TestTaxonomyListRunsSourceIDTriState(t *testing.T) {
ctx := context.Background()
db := setupTaxonomyNoSourceTestDB(t)
repo := repository.NewTaxonomyRepository(db)

tenantID := "taxonomy-listfilter-" + uuid.NewString()
sourceType := "formbricks"
fieldID := "feedback"
sourcedID := "survey-" + uuid.NewString()

t.Cleanup(func() {
_, _ = db.Exec(ctx, `DELETE FROM taxonomy_runs WHERE tenant_id = $1`, tenantID)
})

// One run with no source ("") and one with a concrete source.
for _, sourceID := range []string{"", sourcedID} {
_, created, err := repo.CreateRunIfAvailable(ctx, repository.CreateTaxonomyRunParams{
TaxonomyScope: models.TaxonomyScope{
TenantID: tenantID,
SourceType: sourceType,
SourceID: sourceID,
FieldID: fieldID,
},
})
require.NoError(t, err)
require.True(t, created)
}

ptr := func(s string) *string { return &s }

// nil filter: both runs.
all, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID})
require.NoError(t, err)
require.Len(t, all, 2)

// pointer to "": only the no-source run.
noSource, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID, SourceID: ptr("")})
require.NoError(t, err)
require.Len(t, noSource, 1)
require.Empty(t, noSource[0].SourceID)

// pointer to a concrete value: only that sourced run.
sourced, err := repo.ListRuns(ctx, models.ListTaxonomyRunsFilters{TenantID: tenantID, SourceID: ptr(sourcedID)})
require.NoError(t, err)
require.Len(t, sourced, 1)
require.Equal(t, sourcedID, sourced[0].SourceID)
}
Loading