From e5b6a1b69687fe42e7e401b843bab1366faef698 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 07:44:03 -0400 Subject: [PATCH 1/7] docs(feedback): spec set for Feedback Aggregation & Clustering System MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advance the Feedback Aggregation & Clustering System project through its spec phase. The dimensional-model design (previously drafted but never committed) is recovered and joined by concrete, repo-grounded specs that resolve the open items it handed to downstream tasks. Why: the RFC (mitodl/hq#12210) settled the two architecture decisions (contract-first interim landing; one common tfact_feedback). This turns that direction into build-ready specs so implementation can start without re-deriving source columns, dbt conventions, or the ML/orchestration shape. Adds under docs/design/: - README_feedback_aggregation.md — spec index + key decisions + phasing - feedback_dimensional_model.md — recovered schema design (tfact_feedback) - feedback_event_contract_spec.md — common event contract + business keys - feedback_zendesk_mvp_spec.md — build-ready Zendesk MVP dbt models/columns - feedback_ml_approach.md — embedding/clustering/category/sentiment - feedback_dagster_asset_spec.md — batch ML asset (clones student_risk_probability) - feedback_consumption_ux_spec.md — audiences, Superset surfaces, access control Design/tracking only; no pipeline code. MVP ships the fact first; ML is an additive consumer. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RJXxq9oEabCbJiXFzSTqkY --- docs/design/README_feedback_aggregation.md | 59 +++++ docs/design/feedback_consumption_ux_spec.md | 101 ++++++++ docs/design/feedback_dagster_asset_spec.md | 182 +++++++++++++++ docs/design/feedback_dimensional_model.md | 222 ++++++++++++++++++ docs/design/feedback_event_contract_spec.md | 105 +++++++++ docs/design/feedback_ml_approach.md | 181 +++++++++++++++ docs/design/feedback_zendesk_mvp_spec.md | 243 ++++++++++++++++++++ 7 files changed, 1093 insertions(+) create mode 100644 docs/design/README_feedback_aggregation.md create mode 100644 docs/design/feedback_consumption_ux_spec.md create mode 100644 docs/design/feedback_dagster_asset_spec.md create mode 100644 docs/design/feedback_dimensional_model.md create mode 100644 docs/design/feedback_event_contract_spec.md create mode 100644 docs/design/feedback_ml_approach.md create mode 100644 docs/design/feedback_zendesk_mvp_spec.md diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md new file mode 100644 index 000000000..c07e74665 --- /dev/null +++ b/docs/design/README_feedback_aggregation.md @@ -0,0 +1,59 @@ +# Feedback Aggregation & Clustering System — Spec Index + +Project: `wp-feedback-aggregation-clustering-system-2e9750` · Phase: **spec** (2026-07-10) +RFC (team review): [mitodl/hq#12210](https://github.com/mitodl/hq/discussions/12210) +(supersedes [#10793](https://github.com/mitodl/hq/discussions/10793)). + +A feedback aggregation system that ingests free-text feedback from multiple sources +(Zendesk, edX forum, Learn AI tutor, ORA, a forthcoming edX feedback plugin) into one +source-agnostic dimensional fact and clusters it to surface systemic issues and positive +signals for four audiences (support, engineering, instructors, leadership). + +## The spec set + +| Doc | Scope | Resolves task | +|---|---|---| +| [`feedback_dimensional_model.md`](./feedback_dimensional_model.md) | Dimensional model: `tfact_feedback` grain, conformed dims, sparse FKs, PII redaction, phasing | schema design (discovery) | +| [`feedback_event_contract_spec.md`](./feedback_event_contract_spec.md) | Common feedback event contract + migration-proof business keys; data-bus alignment as a bounded dependency | `...contract-bus-245a8e` | +| [`feedback_zendesk_mvp_spec.md`](./feedback_zendesk_mvp_spec.md) | Build-ready Zendesk MVP: exact dbt models/columns `int__feedback__zendesk → __unioned → tfact_feedback` + 3 dims + tests | (MVP implementation) | +| [`feedback_ml_approach.md`](./feedback_ml_approach.md) | Embedding (local, PII-safe), clustering (UMAP+HDBSCAN), category discovery (seed + LLM-label), sentiment (explicit + kNN/classifier) | `...clustering-...a1d7d6`, `...category-...550aba`, `...sentiment-...92988e` | +| [`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md) | Batch ML Dagster asset cloned from `student_risk_probability`; Vault-backed LLM resource; net-new deps; scheduling | (ML pipeline orchestration) | +| [`feedback_consumption_ux_spec.md`](./feedback_consumption_ux_spec.md) | Audiences × altitude, Superset-first surfaces, per-persona actions, access control | `...ui-ux-...476d23` | + +## Key decisions (spec phase) + +1. **Contract-first, interim landing** (RFC Option 3): ship on the learn-ai landing now, + migrate to the analytics-api/StarRocks data bus later — durable artifact is the **event + contract + business keys**, so migration = source-swap + backfill + parity. +2. **One common `tfact_feedback`** conforming to the Kimball layer (mirrors + `tfact_discussion_events`), with **sparse nullable conformed FKs** as the source-flexibility + mechanism. Explicit **stable `feedback_pk`** (diverges from precedent) for migration + late-arriving + category/sentiment updates. +3. **ML is an additive consumer**, not a prerequisite: the fact ships useful with + tag-seeded categories + CSAT-derived sentiment; embeddings/clustering fill `category_fk`/ + `sentiment_fk`/`embedding_id` later. Embeddings persisted **once** (the one adopted lesson + from prototype #10793). +4. **Local, PII-safe embeddings** by default (learner-PII posture); redaction (Presidio) + mandatory pre-embed; raw text stays upstream under existing Lakekeeper/Cedar authz. +5. **Superset-first consumption**; support + engineering are the MVP-served audiences + (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). + +## Phasing + +- **MVP (Phase 1):** Zendesk-only fact + 3 dims + support/eng cluster dashboards. ~198K rows, batch. +- **Phase 2:** add forum/tutor/ORA (additive CTEs); `afact_feedback_cluster_daily`; instructor course views. +- **Phase 3:** migrate ingress to the data bus (gated on the write path existing + sink-topology decision). + +## Open questions carried to team / platform (non-blocking for MVP) + +- Data-bus write path (net-new platform work) + generic-sink-vs-per-topic (platform-wide, outranks feedback). +- Final embedding model + vector store at full 1.18M scale (MVP proves it at 198K). +- Which prototype #10793 mechanics earn their place (semantic-summary-before-embedding, + hierarchical truncation) — test as hypotheses, don't inherit. +- Qualtrics/course-survey onboarding; HubSpot NPS tickets need Iceberg modeling first. + +## Status / next + +RFC #12210 is posted for team review (Draft; no comments yet as of 2026-07-10). On team +consensus, flip RFC → Accepted and begin implementation per the build order in +`feedback_dagster_asset_spec.md` §7 (dbt fact first, ML asset additive). diff --git a/docs/design/feedback_consumption_ux_spec.md b/docs/design/feedback_consumption_ux_spec.md new file mode 100644 index 000000000..3de34f586 --- /dev/null +++ b/docs/design/feedback_consumption_ux_spec.md @@ -0,0 +1,101 @@ +# Feedback Aggregation — Consumption / UX Spec + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Resolves `tk-...-ui-ux-audience-actions-and-where-the-expe-476d23` +Companion to [`feedback_dimensional_model.md`](./feedback_dimensional_model.md). + +Defines **who consumes the feedback fact, at what altitude, what actions they take, and +where the experience lives.** The fact + dims are the substrate; this spec is the demand +side that validates the model actually serves its audiences. + +--- + +## 1. Audiences × altitude (from the RFC) + +Four audiences at three altitudes. Each maps to a specific slice of `tfact_feedback` + +`dim_feedback_category`/`dim_sentiment` and a specific row-level access rule. + +| Audience | Altitude | Core question | Grain they work at | Row-level access | +|---|---|---|---|---| +| **Customer support** | operational | "What recurring issues are coming in this week?" | cluster → individual ticket drill-down | all feedback | +| **Engineering** | operational | "Which systemic platform defects does feedback point to?" | cluster (systemic-only, filter noise) → ticket | all feedback | +| **Instructors / instructional designers** | course-scoped | "What are my learners saying about *my* course?" | feedback filtered to their `courserun_fk` | only their courserun(s) | +| **Department leadership** | strategic | "How are category/sentiment trends moving; where are new-offering opportunities?" | `afact_feedback_cluster_daily` rollups | aggregates only (no raw text) | + +Three altitudes = the same fact read three ways: **operational drill-down** (row-level, +support/eng), **course-scoped** (filtered fact, instructors), **strategic rollup** +(aggregate fact, leadership). This is why `dim_feedback_source.source_audience_scope` +(`operational | course | strategic`) exists on the source dim. + +--- + +## 2. Surface — where the experience lives + +**Recommendation: start in Superset (BI), not a custom app.** Rationale: +- The warehouse already exposes facts to Superset; zero new app infrastructure (matches the + RFC's "effectively zero new infrastructure" MVP posture). +- Support/eng/leadership are already BI consumers; instructors are the only audience that + might warrant an embedded surface later. +- Row-level security in Superset covers the instructor courserun-scoping and the + leadership aggregates-only rule. + +**MVP surfaces (Superset dashboards):** +1. **Support triage** — cluster list ranked by size/recency, filterable by + `source_status`/`source_priority`/`source_channel`/`source_tags`, each cluster + drill-through to constituent (redacted) tickets with `source_url` deep-links back to + Zendesk. Sentiment facet from `dim_sentiment`. +2. **Engineering systemic view** — same cluster list but filtered to systemic clusters + (HDBSCAN non-noise, above a `min_cluster_size` threshold; noise/one-offs hidden), sorted + by cluster cohesion/size, with category labels from `dim_feedback_category`. +3. **Leadership trends** — `afact_feedback_cluster_daily` time-series of category × sentiment + volume; no raw text. (Phase 2, needs the aggregate fact + cross-source data.) +4. **Instructor course view** — feedback filtered to a courserun; **Phase 2** (needs + course-scoped sources: forum/tutor/ORA — Zendesk MVP is not course-scoped, so instructors + get nothing from the MVP slice. Documented gap.) + +**When to graduate to an embedded/custom app:** only if write-back actions (§3) outgrow +Superset — e.g. support wants in-context triage/routing, or eng wants one-click issue +filing. Treat that as a deliberate later decision, not MVP. + +--- + +## 3. Actions per persona (write-back) + +MVP is **read-only insight**; write-back is explicitly deferred but the design should not +preclude it. Documented target actions (open question, RFC): + +| Persona | Read (MVP) | Write-back (later) | +|---|---|---| +| Support | view clusters, drill to tickets, filter | mark cluster as triaged / route to team | +| Engineering | view systemic clusters, category labels | file a linked eng issue from a cluster | +| Instructor | (Phase 2) view own-course feedback | acknowledge / respond | +| Leadership | view trends | none (consumption only) | +| Curator (any) | — | approve/merge/deprecate `dim_feedback_category` labels (the curation loop) | + +The **category curation loop** (approve LLM-proposed labels) is the one write-back that IS +needed early — it can live as a lightweight admin surface or even a manual dbt-seed edit at +MVP, upgrading to a UI when volume warrants (`feedback_ml_approach.md` §D). + +--- + +## 4. Access control (ties to design §7 + Lakekeeper/Cedar) + +- **Redacted text only** is exposed in any consumption surface (raw text stays in + `raw`/`stg` under existing PII classification + Lakekeeper/Cedar authz — design §7). +- **Instructor** dashboards are row-filtered to their `courserun_fk` (Superset RLS or a + Cedar policy on the fact). +- **Leadership** sees the aggregate fact only (no row-level text) — enforced by pointing + their dashboards at `afact_feedback_cluster_daily`, not `tfact_feedback`. +- Re-applying PII classification + Lakekeeper/Cedar authz at the new fact/dims is tracked by + `tk-re-apply-pii-classification-lakekeeper-cedar-aut-1b3516` (p3). + +--- + +## 5. What the MVP consumption slice actually delivers + +Given the MVP is Zendesk-only and not course-scoped: **support + engineering get working +cluster-triage dashboards; leadership gets Zendesk-only category/sentiment trends; +instructors get nothing until Phase 2 course-scoped sources land.** That is the honest +scope — the model supports all four, but only two-and-a-half audiences are served by the +first data slice. This is acceptable because support/eng recurring-issue triage is the +primary stated motivation. diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md new file mode 100644 index 000000000..6eca2797f --- /dev/null +++ b/docs/design/feedback_dagster_asset_spec.md @@ -0,0 +1,182 @@ +# Feedback Aggregation — Dagster ML Asset Spec (MVP) + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Companion to [`feedback_zendesk_mvp_spec.md`](./feedback_zendesk_mvp_spec.md) +and [`feedback_ml_approach.md`](./feedback_ml_approach.md) + +The scheduled batch job that turns the redacted feedback fact into embeddings, clusters, +LLM-proposed categories, and sentiment. Grounded in the existing repo orchestration +patterns. **The fact ships without this asset**; this is purely additive (fills +`embedding_id`, `category_fk`, `sentiment_fk`, and the `feedback_embeddings` sidecar). + +--- + +## 1. Template: clone `student_risk_probability` + +There is a near-exact precedent code location: `dg_projects/student_risk_probability/` — a +standalone `dg`-managed Dagster project whose single Python `@asset` reads a dbt-modeled +Iceberg table into a Polars frame, runs scikit-learn ML, and writes results back via the +Iceberg IO manager. **Clone its structure** for a new +`dg_projects/feedback_clustering/` code location: + +``` +dg_projects/feedback_clustering/ + pyproject.toml # + NEW deps: embeddings + clustering + LLM + presidio (§4) + uv.lock + Dockerfile + build.yaml + feedback_clustering/ + definitions.py # Definitions(assets, resources={io_manager, vault, llm}, jobs, schedules) + assets/feedback_clustering.py # thin @asset(s) + resources/llm.py # NEW Vault-backed LLM/embeddings client factory + lib/embed.py # embedding + redaction helpers + lib/cluster.py # UMAP+HDBSCAN helpers + lib/label.py # LLM cluster-labeling + sentiment helpers + schedules/ # cron ScheduleDefinition (optional; declarative default) +``` + +Scaffold via the `dagster-code-location-structure` skill (`dg`), copying +`student_risk_probability`'s `definitions.py` (Iceberg `PolarsIcebergIOManager`, Vault +auth, `define_asset_job`) verbatim as the skeleton. Replicate its Python pin +(`>=3.14,<3.15`) and its `pyiceberg`/`grpcio` overrides. + +--- + +## 2. Asset graph (MVP) + +Model each stage as its own `@asset` so re-runs are granular and cached independently. All +read/write Iceberg via `get_dbt_model_as_dataframe(...)` +(`ol_orchestrate.lib.glue_helper`) and the `io_manager` (`PolarsIcebergIOManager`). + +``` +[dbt] int__feedback__unioned (redacted text + feedback_pk) ← upstream AssetKey dep + │ + ▼ +feedback_embeddings @asset → writes feedback_embeddings (feedback_pk, vector, + │ model_version) [embedding computed ONCE per model_version] + ▼ +feedback_clusters @asset → adds cluster_id, cluster_probability, cluster_run_id + │ to feedback_embeddings (UMAP→HDBSCAN) + ├─────────────► feedback_category_proposals @asset → LLM-labels clusters → dim_feedback_category + │ (category_source='llm_discovered', status='proposed') + └─────────────► feedback_sentiment @asset → sentiment per feedback_pk (explicit + kNN/classifier) + → writes sentiment_fk assignments + ▼ +tfact_feedback late-arriving update ← category_fk / sentiment_fk / embedding_id upsert by feedback_pk +``` + +- **Redaction placement (design §7 / MVP spec §3):** the `feedback_embeddings` asset does + the Presidio redaction on its input (or reads an already-redacted `int__feedback__unioned` + column). Recommend redaction lives here in Python since Presidio is Python — the fact then + reads redacted text produced by this asset. **Decision for implementation:** either (a) + the asset writes `text_redacted` back to a table the fact reads, or (b) `int__feedback__unioned` + is itself a Python-materialized step. Pick (a) to keep dbt pure-SQL; documented as the one + interleave point. +- **`code_version`** on each asset (as `student_risk_probability` does) so a helper/model + change re-triggers via declarative automation. +- **`pool=`** set per asset (concurrency governed by the production instance pool config — + slot limits live in the Dagster UI, not repo config). + +--- + +## 3. Reading & writing data (exact repo helpers) + +- **Read** the dbt fact/union: `get_dbt_model_as_dataframe(database_name="ol_warehouse_production_", + table_name="int__feedback__unioned")` → Polars. (Same call `student_risk_probability` + uses against `reporting.cheating_detection_report`.) +- **Write** results: return a `pl.DataFrame` from the asset; the `io_manager` key + (`PolarsIcebergIOManager`, configured in `definitions.py`) persists it to the target + Iceberg table. `feedback_embeddings` is a new Iceberg table with the schema in + `feedback_ml_approach.md` §B/§C. +- **Late-arriving `category_fk`/`sentiment_fk` back onto `tfact_feedback`:** because dbt owns + `tfact_feedback`, the cleanest MVP path is an incremental dbt model / `merge` keyed by + `feedback_pk` that reads the assignment table this asset writes — not a direct Python + mutation of the fact. So the asset writes `feedback_category_assignments` / + `feedback_sentiment_assignments` Iceberg tables, and a dbt step joins them onto the fact. + This keeps the fact dbt-owned and the ML output append-only. + +--- + +## 4. New dependencies (net-new to the repo) + +Confirmed absent repo-wide today (no openai/anthropic/sentence-transformers/transformers/ +torch/hdbscan/umap/faiss/qdrant/pgvector anywhere). Add to the **new project's** +`pyproject.toml` only (keep them out of the shared lib and other locations): + +| Purpose | Dep | Notes | +|---|---|---| +| Embeddings (local, PII-safe) | `sentence-transformers` (pulls `torch`) | default; CPU ok at MVP. Heavy image — consider a CPU-only torch wheel. | +| Dim-reduction + clustering | `umap-learn`, `hdbscan` | or use in-stack `scikit-learn` `HDBSCAN`≥1.3 to avoid `hdbscan` compiled dep — decide on image build constraints. `scikit-learn` already proven in-stack. | +| LLM cluster labeling + (fallback) sentiment | Anthropic client (Claude Haiku/Sonnet class) | one call per *cluster*, not per record — cheap batch. | +| PII redaction | `presidio-analyzer`, `presidio-anonymizer` (+ spaCy model) | precedent: OM profiler already runs Presidio recognizers. | + +**Image-size caveat:** `torch` + spaCy make a large image. If that is a problem, the +fallback is a hosted embedding API behind the same resource interface (accepting the +PII-egress tradeoff called out in `feedback_ml_approach.md` §B) — but default to local. + +--- + +## 5. Secrets — Vault `ConfigurableResource` (repo pattern) + +API keys come from **HashiCorp Vault via a `ConfigurableResource`**, not `EnvVar` +(repo convention; `EnvVar` is not used for secrets here). Build an +`LLMClientFactory(ConfigurableResource)` holding `vault: Vault`, modeled exactly on +`packages/ol-orchestrate-lib/src/ol_orchestrate/resources/github.py`: + +```python +class LLMClientFactory(ConfigurableResource): + vault: Vault = Field(...) + vault_mount_point: str = Field(default="secret-data") + vault_secret_path: str = Field(default="pipelines/feedback-llm") # NEW Vault path to provision + vault_secret_key: str = Field(default="api_key") + _client = PrivateAttr(default=None) + + def get_client(self): + if self._client is None: + data = self.vault.client.secrets.kv.v1.read_secret( + mount_point=self.vault_mount_point, path=self.vault_secret_path) + self._client = Anthropic(api_key=data["data"][self.vault_secret_key]) + return self._client +``` + +Register in `Definitions.resources` alongside the `vault` resource +(`authenticate_vault(DAGSTER_ENV, VAULT_ADDRESS)` with the resilient-load fallback, as in +`student_risk_probability/definitions.py`). **Action item:** provision a Vault secret path +(e.g. `pipelines/feedback-llm`) for the LLM key. A local embedding model needs no secret. + +--- + +## 6. Scheduling + +Two options, both in use in the repo: +- **(recommend) Declarative automation:** put `automation_condition=upstream_or_code_changes()` + (`ol_orchestrate.lib.automation_policies`) on the `feedback_embeddings` asset so it re-runs + when `int__feedback__unioned` refreshes or the code version changes. Downstream cluster/ + label/sentiment assets chain off it. This is what `student_risk_probability` and the dbt + assets use — no cron to maintain. +- **Cron alternative:** wrap the assets in `define_asset_job(...)` + a + `dg.ScheduleDefinition(cron_schedule="0 4 * * *", execution_timezone="Etc/UTC")` (pattern: + `dg_projects/data_loading/.../schedules.py`) if a fixed cadence is preferred over + data-driven triggering. MVP volume (~198K) runs comfortably in one nightly batch. + +--- + +## 7. Build order (so the fact ships first) + +1. dbt models (`feedback_zendesk_mvp_spec.md`) — `tfact_feedback` + dims. **Ships and is + useful with tag-seeded categories + CSAT-derived sentiment, no ML.** +2. Scaffold `dg_projects/feedback_clustering/`; add deps; provision Vault path. +3. `feedback_embeddings` asset (embed + redact) → sidecar table. +4. `feedback_clusters` asset (UMAP+HDBSCAN). +5. `feedback_category_proposals` + `feedback_sentiment` assets → assignment tables. +6. dbt late-arriving join of assignment tables onto `tfact_feedback` (`category_fk`/`sentiment_fk`). +7. Human curation loop on `dim_feedback_category` (approve/merge proposed labels). + +--- + +## 8. Explicit non-goals (MVP) + +- No online/real-time embedding or serving (batch only). +- No dedicated vector DB (Iceberg `ARRAY` sidecar; revisit at Phase 2/serving need). +- No GPU requirement (CPU batch at MVP scale; revisit at full 1.18M-row scale). +- No cross-source clustering until forum/tutor/ORA sources land (Phase 2). diff --git a/docs/design/feedback_dimensional_model.md b/docs/design/feedback_dimensional_model.md new file mode 100644 index 000000000..9aaa822ea --- /dev/null +++ b/docs/design/feedback_dimensional_model.md @@ -0,0 +1,222 @@ +# Feedback Aggregation — Dimensional Model Design + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-06 (schema) · See [`README_feedback_aggregation.md`](./README_feedback_aggregation.md) for the full spec set. + +Conforms to the existing Kimball layer in `src/ol_dbt/models/dimensional` (`tfact_*` transactional facts, +`afact_*` aggregate facts, `dim_*` conformed dimensions, surrogate `*_pk` via +`dbt_utils.generate_surrogate_key`, `*_fk` in facts, `time_fk`/`date_fk`). Precedents: `tfact_discussion_events`, +`tfact_chatbot_events`. + +--- + +## 1. Grain + +**`tfact_feedback` — one row per atomic free-text feedback utterance.** + +"Utterance" = the smallest independently-clusterable unit of a source: + +| Source | One row = | Text column source | +|--------|-----------|--------------------| +| Zendesk | one ticket (first comment) | `ticket_description` (+ `ticket_subject`) | +| edX forum | one `*.created` post/response/comment | `post_content` (+ `post_title`) | +| Learn AI tutor | one human turn | `human_message` | +| ORA peer feedback | one feedback submission | `feedback_text` | +| edX feedback plugin (new) | one feedback event | `feedback_text` (contract §5) | + +Rationale: the ML tasks (embed → cluster → label) operate per-utterance. Ticket *threads* and full tutor +*conversations* are deliberately **not** the grain — a `conversation_fk` / `thread_id` degenerate key preserves +the ability to roll up. Agent/bot messages are excluded (not feedback); forum view/vote/follow events are +excluded (filter to `*.created`). + +--- + +## 2. The fact: `tfact_feedback` + +``` +-- surrogate + conformed FKs +feedback_pk -- generate_surrogate_key([feedback_source, source_natural_key]) (see §6) +feedback_source_fk -> dim_feedback_source.feedback_source_pk +user_fk -> dim_user.user_pk (nullable; anonymous/aggregate sources) +platform_fk -> dim_platform.platform_pk (nullable; non-course sources e.g. Zendesk) +courserun_fk -> dim_course_run.courserun_pk (nullable; only course-scoped sources) +organization_fk -> dim_organization.organization_pk (nullable; Zendesk group/org) +category_fk -> dim_feedback_category.feedback_category_pk (late-arriving, §4a) +sentiment_fk -> dim_sentiment.sentiment_pk (late-arriving, §4b) +time_fk -> dim_time +date_fk -> dim_date + +-- degenerate / conversational keys +conversation_id -- thread/ticket id, source-native (roll up utterances → conversation) +source_record_id -- source PK (zendesk ticket_id, forum post_id, checkpoint id, ora id) +source_url -- deep link back to origin (ticket api url, forum page_url) + +-- text (REDACTED — see §7; raw text never lands in the fact) +feedback_title -- subject / post_title (redacted) +feedback_text -- description / post_content / human_message / feedback_text (redacted) +feedback_text_chars -- length metric (pre-redaction), for sizing/analytics +embedding_id -- FK/handle into the embedding + cluster store (§4c); nullable until scored + +-- source-native descriptive attributes (kept for facet filters; NOT conformed) +source_status -- zendesk ticket_status / null +source_priority -- zendesk ticket_priority / null +source_tags -- array: zendesk ticket_tags, forum roles, etc. (category SEEDS, §4a) +source_channel -- zendesk source_channel / tutor agent / forum component +csat_score -- zendesk satisfaction_rating_score / tutor rating / null (explicit sentiment) + +-- audit +feedback_occurred_at -- source event time (created_at / event_timestamp / checkpoint_created_on) +feedback_ingested_at +``` + +Materialized `table`. Nullable FKs are expected and correct: Zendesk has no courserun; forum/tutor have no +org; anonymous feedback has no user. The fact tolerates sparse conformance — that is what makes it +*source-flexible*. + +--- + +## 3. Conformed dimensions — REUSED (no new work) + +| Dim | Key | Populated for | +|-----|-----|---------------| +| `dim_user` | `user_pk` | forum, tutor, ORA, Zendesk (where requester resolves) | +| `dim_platform` | `platform_pk` | course-scoped sources | +| `dim_course_run` | `courserun_pk` | forum, tutor, ORA (via `courserun_readable_id`) | +| `dim_organization` | `organization_pk` | Zendesk (`group_name`/`organization_name`) | +| `dim_date` / `dim_time` | `date_fk`/`time_fk` | all | + +**Identity is the highest-risk join** (cf. open p0 bug `tk-fix-dim-user-null-email-identity-collapse`). +Resolve `user_fk` via the same paths the existing facts use — `openedx_user_id` → `dim_user.mitlearn_openedx_user_id` +(forum/tutor), `user_global_id` (tutor `int__learn_ai__chatbot`), email (Zendesk requester, last resort). +Never key the fact off a source's local PK (§6). + +--- + +## 4. New dimensions + +### 4a. `dim_feedback_category` (LLM/embedding-discovered, curated) +``` +feedback_category_pk -- generate_surrogate_key([category_slug]) +category_slug -- stable machine key (survives relabeling) +category_label -- human label (LLM-proposed, human-approved) +category_parent_slug -- optional hierarchy (cluster → category → theme) +category_status -- proposed | approved | merged | deprecated +category_source -- 'llm_discovered' | 'seed' | 'manual' +first_seen_at / updated_at +``` +Bootstrapped, **not cold-start**: seed from Zendesk `ticket_tags` (2,354 distinct) + `group_name`, then LLM-label +embedding clusters (task `tk-define-llm-driven-category-discovery`). SCD-lite: relabeling changes `category_label`, +never `category_slug`. Assignment (`category_fk` on the fact) is late-arriving — a ticket can be categorized after +insert. + +### 4b. `dim_sentiment` +``` +sentiment_pk -- generate_surrogate_key([sentiment_slug]) +sentiment_slug -- 'positive' | 'neutral' | 'negative' | (aspect-based later) +polarity_score_bucket -- optional coarse bucket for trend rollups +``` +Small conformed dim. `sentiment_fk` derived by the sentiment task (`tk-define-sentiment-mapping`) — +semantic/embedding or LLM. Explicit signals (`csat_score`, tutor `rating`) seed/validate it. + +### 4c. `dim_feedback_source` +``` +feedback_source_pk -- generate_surrogate_key([source_slug]) +source_slug -- 'zendesk' | 'edx_forum' | 'learn_ai_tutor' | 'ora' | 'edx_feedback_plugin' +source_name -- display +source_medium -- 'support_ticket' | 'forum_post' | 'chat' | 'assessment' | 'in_product' +source_audience_scope -- 'operational' | 'course' | 'strategic' (see audience memory) +is_course_scoped -- bool (whether courserun_fk applies) +``` + +### 4d. Embedding + cluster store (NOT in the dimensional schema) +Vectors and cluster assignments live in a **sidecar** table (`feedback_embeddings`) keyed by `embedding_id`, +not in `tfact_feedback` (facts stay narrow, embeddings churn on re-clustering). It holds: `embedding_id`, +`feedback_pk`, `vector`, `cluster_id`, `cluster_run_id`, `model_version`. `dim_feedback_category` is the +*curated* projection of stable clusters; the sidecar is the *mutable* ML working set. This keeps re-clustering +from rewriting the fact. + +--- + +## 5. Common feedback event contract (interim↔target bridge) + +Both the interim learn-ai landing (`raw__learn_ai__…__feedback`) and the eventual analytics-api/StarRocks +data-bus MUST emit this shape so `stg__…__feedback` → `tfact_feedback` is source-path-agnostic +(task `tk-define-stable-common-feedback-event-contract-bus-245a8e`): + +``` +source_slug -- maps to dim_feedback_source +occurred_at -- ISO8601 +subject_user_ref -- global/openedx user id (NOT a source-local PK) +courserun_readable_id -- nullable +platform -- nullable +conversation_ref -- thread/ticket id +source_record_ref -- source-native id (idempotency key) +title / text -- raw free text (redaction happens in-warehouse, §7) +source_metadata -- json: tags, status, priority, channel, csat, etc. +``` +Align this to the **general data-bus ingestion contract**, not a feedback-specific one, so future openedx +tracking logs share ingress semantics. + +--- + +## 6. Business-key strategy (migration-proof) + +`feedback_pk = generate_surrogate_key([feedback_source, source_natural_key])` where `source_natural_key` is a +**stable business identifier from the source system**, never a warehouse/Airbyte/Postgres row PK: + +| Source | source_natural_key | +|--------|--------------------| +| Zendesk | `ticket_id` | +| edX forum | `post_id` | +| Learn AI tutor | `thread_id` + `checkpoint_pk` (or message index) | +| ORA | `submission_uuid` | +| edX plugin | contract `source_record_ref` | + +This is what lets the interim→analytics-api migration be "swap source + backfill + parity" rather than a +rebuild: the same `feedback_pk` regenerates identically regardless of pipe. + +--- + +## 7. PII handling (mandatory pre-embed) + +`ticket_description` and `int__learn_ai__chatbot.human_message` carry emails/phones/names (profiler-flagged +`PII.Sensitive`). A **redaction step in the `int__` layer** (Presidio — precedent: the OM profiler already runs +Presidio recognizers) masks entities before text lands in `tfact_feedback` or the embedding store. Raw text +stays in `raw`/`stg` under existing PII classification + Lakekeeper/Cedar authz; the fact carries redacted text +only. Access to the fact is still governed per §audience (course instructors see their courserun; support/eng +see tickets; leadership sees aggregates). + +--- + +## 8. Layering & build path + +``` +raw____… (existing per source; new: raw__learn_ai__…__feedback) + → stg____…__feedback (conform to §5 contract, light typing) + → int__feedback__unioned (UNION all sources into the common shape + redact PII §7) + → tfact_feedback (resolve conformed FKs, generate feedback_pk §6) + → afact_feedback_cluster_daily (aggregate fact: cluster × category × sentiment × date × source — strategic rollup) +``` +Clustering/labeling jobs (Dagster asset) read `int__feedback__unioned` (redacted) → write `feedback_embeddings` ++ `dim_feedback_category` + `category_fk`/`sentiment_fk` back onto the fact (late-arriving update). + +--- + +## 9. MVP vs. full + +- **MVP (support/eng):** `dim_feedback_source` + `tfact_feedback` restricted to `source_slug='zendesk'`, category + seeded from `ticket_tags`, sentiment from `csat_score` + model. ~198K rows, batch. No new dims beyond the three. +- **Phase 2:** add forum/tutor/ORA sources (fact already tolerates them), enable cross-source + `afact_feedback_cluster_daily` for leadership, course-scoped views for instructors. +- **Phase 3:** analytics-api/data-bus ingress (contract §5 already lets the fact ignore the switch). + +--- + +## 10. Open items handed to downstream tasks + +- Category discovery & seeding → `tk-define-llm-driven-category-discovery-to-seed-pop-550aba` +- Sentiment method & `dim_sentiment` grain → `tk-define-sentiment-mapping-via-semantic-embedding--92988e` +- Clustering method + `feedback_embeddings` store + roll-up hierarchy → `tk-define-clustering-approach-for-systemic-issue-de-a1d7d6` +- Per-persona actions, surface, row-level access → `tk-define-ui-ux-audience-actions-and-where-the-expe-476d23` +- Contract finalization + business keys → `tk-define-stable-common-feedback-event-contract-bus-245a8e` +``` diff --git a/docs/design/feedback_event_contract_spec.md b/docs/design/feedback_event_contract_spec.md new file mode 100644 index 000000000..42aba922b --- /dev/null +++ b/docs/design/feedback_event_contract_spec.md @@ -0,0 +1,105 @@ +# Feedback Aggregation — Common Event Contract & Business-Key Strategy + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Resolves `tk-...-common-feedback-event-contract-bus-245a8e` +Consolidates design §5–6; companion to [`feedback_dimensional_model.md`](./feedback_dimensional_model.md). + +This is the **highest-leverage migration de-risker** (do at spec time regardless of interim +path). If the interim learn-ai landing AND the eventual analytics-api/StarRocks data-bus +both emit this shape, the migration collapses to *source-swap + backfill + parity*; if they +diverge, it's a rebuild. + +--- + +## 1. The contract (source-agnostic feedback event) + +Every producer — the interim learn-ai feedback table, the edX feedback plugin, and each dbt +staging model that adapts an existing source (Zendesk, forum, tutor, ORA) — MUST present +this shape at the `stg__…__feedback` boundary: + +| Field | Type | Meaning | Required | +|---|---|---|---| +| `source_slug` | string | maps to `dim_feedback_source.source_slug` | yes | +| `occurred_at` | ISO8601 timestamp | source event time | yes | +| `source_record_ref` | string | **stable source-native id** — idempotency key + business key (§2) | yes | +| `text` | string | raw free text (redaction happens in-warehouse, design §7) | yes | +| `title` | string | subject/heading; nullable | no | +| `conversation_ref` | string | thread/ticket id — roll utterances → conversation | no | +| `subject_user_ref` | string | **global/openedx user id** (NOT a source-local PK); email only as last resort | no | +| `courserun_readable_id` | string | course scope; null for non-course sources | no | +| `platform` | string | platform readable id; nullable | no | +| `source_metadata` | JSON | tags, status, priority, channel, csat, etc. | no | + +**Design rules:** +- `subject_user_ref` is a **global identity ref, never a source row PK** — this is what lets + `user_fk` resolve against `dim_user` consistently across sources and survive the migration. + (Zendesk, lacking an openedx id, uses requester email as the documented last-resort ref — + which shares the `dim_user` NULL-email identity-collapse failure class; see design §3.) +- `text`/`title` carry **raw** text across the contract; redaction is a warehouse step + (design §7), not a producer responsibility — so producers never need Presidio. +- `source_metadata` is the extension point: anything source-specific rides in the JSON and is + projected into the fact's non-conformed facet columns (`source_status`, `source_priority`, + `source_tags`, `source_channel`, `csat_score`) without changing the contract. + +--- + +## 2. Business-key strategy (migration-proof) + +``` +feedback_pk = generate_surrogate_key([source_slug, source_record_ref]) +``` + +`source_record_ref` is a **stable business identifier from the source system**, never a +warehouse/Airbyte/Postgres row PK: + +| Source | `source_record_ref` | +|---|---| +| Zendesk | `ticket_id` | +| edX forum | `post_id` | +| Learn AI tutor | `thread_id` + `checkpoint_pk` (or message index) | +| ORA | `submission_uuid` | +| edX feedback plugin | plugin-native event/record id | + +Because `feedback_pk` regenerates **identically** regardless of which pipe delivered the +event, the interim→data-bus swap re-lands the same rows with the same keys — enabling +backfill + parity validation instead of a rebuild. This is the linchpin of the whole +contract-first strategy (RFC Option 3). + +--- + +## 3. Alignment to the general data-bus ingestion contract (dependency, not a decision here) + +Per the 2026-07-06 correction: this contract should align to the **general StarRocks +data-bus ingestion contract** (shared write/dedup/flush semantics), *not* a feedback-specific +shape — so feedback and the future openedx tracking-logs workload share ingress semantics. + +**But** the data-bus write path does not exist yet, and its sink topology (**one generic +sink vs. sink-per-topic**) is an unsettled platform-wide question that outranks feedback +(`pf-starrocks-data-bus-write-path-not-built-yet-gene-bf78da`; RFC Open Questions). So: + +- This spec defines the **feedback event contract now** (the durable artifact) and keys the + fact off stable business ids (§2) — both fully within feedback's control and sufficient to + de-risk the migration. +- It does **not** unilaterally define the general bus contract. When the platform settles + generic-vs-per-topic and builds the write path, this contract's field set is the proposed + feedback *topic* payload; reconcile field names/envelope with the bus's shared envelope at + that time. The business-key rule (§2) is invariant under that reconciliation. +- **Landing namespace** on eventual flush: a dedicated `feedback`/ingested schema (or a + per-topic schema, depending on the sink decision), **not** `raw`. Decided before migration + step 9, not now. + +**Net:** feedback ships on the contract + business keys defined here (non-blocking for the +MVP); the general-bus alignment is a documented, bounded reconciliation gated on platform +decisions that feedback must conform to rather than set. + +--- + +## 4. Conformance requirements (what each producer/adapter must do) + +- **Interim learn-ai feedback table:** shape its columns to §1 so `stg__learn_ai__…__feedback` + is a near pass-through (RFC Implementation step 2). +- **Zendesk adapter (MVP):** `int__feedback__zendesk` maps ticket columns → §1 + (`feedback_zendesk_mvp_spec.md` §2). `source_record_ref = ticket_id`. +- **Future producers (edX plugin, etc.):** emit §1 directly or via a thin staging adapter. +- **The fact never reads a source's raw shape** — only `int__feedback__unioned` in the §1 + contract shape. This is the isolation that makes new sources additive. diff --git a/docs/design/feedback_ml_approach.md b/docs/design/feedback_ml_approach.md new file mode 100644 index 000000000..1323cff01 --- /dev/null +++ b/docs/design/feedback_ml_approach.md @@ -0,0 +1,181 @@ +# Feedback Aggregation — ML/LLM Approach Spec + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Companion to [`feedback_dimensional_model.md`](./feedback_dimensional_model.md) + +Resolves the open items the dimensional-model design handed to downstream tasks: +clustering (`tk-...-clustering-approach-...a1d7d6`), category discovery +(`tk-...-llm-driven-category-discovery-...550aba`), and sentiment +(`tk-...-sentiment-mapping-...92988e`). Grounded in the discovery cost model +(~1.18M text records; one-time embed ≈ $2–16; MVP = ~198K Zendesk tickets, batch). + +The guiding principle from the RFC: **the durable artifact is the feedback fact + +persisted embeddings; clustering/category/sentiment are re-runnable consumers of it.** +Nothing here rewrites `tfact_feedback`; all ML output lands in the `feedback_embeddings` +sidecar or the late-arriving `category_fk`/`sentiment_fk` update. + +--- + +## A. Pipeline shape (single batch Dagster asset graph, MVP) + +``` +int__feedback__unioned (redacted text, feedback_pk) [dbt] + → embed : text → vector, persisted once [py asset: feedback_embeddings] + → cluster : vectors → cluster_id per cluster_run [py asset: feedback_embeddings.cluster_id] + → label : cluster centroid/samples → category label [py asset: dim_feedback_category (proposed)] + → sentiment : text/vector → sentiment_slug [py asset: sentiment_fk] + → assign : write category_fk/sentiment_fk back [dbt incremental or py upsert] +``` + +Each stage is idempotent and keyed by `feedback_pk` + `model_version`/`cluster_run_id`, +so a re-run never duplicates and never mutates the fact grain. Embedding is computed +**once per (feedback_pk, model_version)**; only cluster/label/sentiment re-run cheaply. + +--- + +## B. Embedding (foundation for clustering AND sentiment) + +**Decision: one shared embedding, computed once, stored in the `feedback_embeddings` +sidecar.** Both clustering and (semantic) sentiment consume it — do not embed twice. + +- **Model choice — recommend an open/self-hostable model** (e.g. `BAAI/bge-small-en-v1.5` + or `sentence-transformers/all-MiniLM-L6-v2`, 384-dim) over a hosted API, for the + **learner-PII posture**: even post-redaction, sending ~1.18M feedback utterances to a + third-party embedding API is an avoidable data-egress and cost exposure. A local + sentence-transformers model runs the full corpus in a single batch job on CPU/GPU for + effectively $0 marginal cost. Keep an OpenAI `text-embedding-3-small` adapter behind the + same resource interface as a fallback/benchmark, but default to local. + - Open question deferred to implementation: GPU vs CPU throughput at 1.18M rows. At + MVP scale (198K) CPU is fine (single-digit minutes to low hours). +- **Redaction is upstream and mandatory** (design §7): embeddings are computed on the + Presidio-redacted text only. Raw text never reaches the embedding step. +- **`model_version` is a first-class column** on `feedback_embeddings`. Changing the model + = new `model_version` rows, old ones retained until re-cluster completes (no in-place + overwrite → no torn state). +- **Vector storage:** at ~1.18M × 384 float32 ≈ 1.8 GB. Store as an Iceberg `ARRAY` + column in `feedback_embeddings` for the MVP (no new operational service; aligns with the + lake). Revisit a dedicated vector index (StarRocks vector column, S3 Vectors, Qdrant) + only when an online nearest-neighbour/serving need appears — the batch clustering job + reads the whole set into memory, which is fine at this scale. This defers the RFC's + "embedding model & vector store" open question to a reversible, low-cost default. + +**Rejected:** re-embedding on every run (the prototype #10793 flaw the RFC explicitly +fixes); semantic-summary-before-embedding as a default (see §C, treated as a hypothesis +to test, not a baseline — an LLM call per record turns a ~$2–16 job into a per-record LLM +cost across 1M+ records and can distort short-text sources). + +--- + +## C. Clustering (systemic-issue detection) — `tk-...-a1d7d6` + +**Goal:** turn per-utterance embeddings into clusters that distinguish a *systemic issue* +(many tickets, one root theme) from a *one-off*. Output = `cluster_id` + a cluster-size / +cohesion signal that lets a human say "this is recurring." + +- **Algorithm: HDBSCAN** (density-based) as the default, over UMAP-reduced embeddings. + Rationale vs. k-means: + - No pre-set `k` — the number of systemic themes is unknown and grows; k-means forces a + guess and splits/merges arbitrarily. + - **Native noise class** — HDBSCAN labels sparse points as noise (`cluster_id = -1`), + which *is* the "one-off complaint" bucket we explicitly want to separate from systemic + signal. This maps directly to the motivation ("not just one-off complaints"). + - Produces a `probability`/`persistence` per point → a cohesion signal for ranking + clusters by how tight/recurring they are. + - Precedent alignment: the #10793 prototype's hierarchical intent is served by + HDBSCAN's condensed tree without baking in its manual truncation. +- **Dimensionality reduction: UMAP** to ~5–15 dims before HDBSCAN (HDBSCAN degrades in + raw high-dim space). `n_neighbors`/`min_cluster_size` are the two knobs to tune on a + Zendesk sample; `min_cluster_size` is effectively the "how many tickets before we call + it systemic" threshold and should be a config, not hardcoded. +- **Re-clustering is cheap and expected:** each run writes a new `cluster_run_id`; the + sidecar keeps prior runs. `dim_feedback_category` (curated) only advances when a human + approves labels from a run (design §4a), decoupling churny clustering from the stable + category dimension. +- **Cross-source clustering (Phase 2):** because all sources share one embedding space in + `int__feedback__unioned`, a cluster can span Zendesk + forum + tutor — this is the + mechanism behind `afact_feedback_cluster_daily` (cluster × category × sentiment × date × + source). No algorithm change needed; just don't filter `source_slug` at cluster time. +- **Deps:** `umap-learn`, `hdbscan` (or `scikit-learn`'s `HDBSCAN` ≥1.3 to avoid the + separate compiled dep — decide at implementation based on the Dagster image's build + constraints). All CPU, no service. + +**Cluster-quality columns** on `feedback_embeddings` are added *only if the chosen +algorithm produces them* (RFC step 6): `cluster_id`, `cluster_probability`, +`cluster_run_id`, `model_version` — silhouette/persistence optional. + +--- + +## D. Category discovery & seeding — `tk-...-550aba` + +**Bootstrapped, not cold-start** (design §4a). Two inputs seed `dim_feedback_category`: + +1. **Seed from existing structure (no LLM):** Zendesk `ticket_tags` (~2,354 distinct) + + support `group_name` give an immediate, human-meaningful starter taxonomy. These become + `category_source='seed'` rows with `category_status='proposed'`. This alone makes the + MVP useful before any clustering runs. +2. **LLM-label the clusters (§C output):** for each HDBSCAN cluster, sample N + representative (redacted) utterances near the centroid + the cluster's dominant seed + tags, and prompt an LLM to propose a short `category_label` + a stable `category_slug` + + a one-line description. `category_source='llm_discovered'`, `category_status='proposed'` + until a human approves (`approved`), merges (`merged`), or deprecates. + +**Key design invariants (from §4a):** +- **SCD-lite on `category_slug`:** relabeling changes `category_label`, never the slug — + so `category_fk` on the fact is stable across renames. +- **Assignment is late-arriving:** a ticket gets `category_fk` after insert, by mapping its + `cluster_id` → the approved category for that cluster. Uncategorized = `category_fk` null + (a valid, queryable state). +- **LLM cost is bounded:** one LLM call *per cluster*, not per record (there are hundreds + of clusters, not millions of tickets). This is the critical cost distinction from the + rejected per-record semantic-summary approach. +- **Model:** any capable instruction model (Claude Haiku/Sonnet class) — labeling a few + hundred clusters is a trivial, cheap batch. Keep the labeler behind a resource interface. + +**Human-in-the-loop is required, not optional:** LLM proposes, a human curates the +category dimension. The dimension is a *curated projection* of clusters (design §4d), which +is why it is a dbt/warehouse dimension and not raw model output. + +--- + +## E. Sentiment mapping — `tk-...-92988e` + +**Grain:** `sentiment_fk` on `tfact_feedback`, one sentiment per utterance. +`dim_sentiment` starts coarse: `positive | neutral | negative` (design §4b), with a +`polarity_score_bucket` for trend rollups. Aspect-based sentiment is a later refinement, +not MVP. + +**Two-tier derivation, cheapest-signal-first:** +1. **Explicit signals seed & validate (free, high-precision):** where the source carries + an explicit rating — Zendesk `satisfaction_rating_score`, tutor `rating`, ORA scores — + map it directly to a sentiment bucket. This gives a labeled validation set for free and + covers a real fraction of rows at zero model cost. +2. **Model the rest:** for utterances with no explicit signal, derive sentiment from the + text. Two options, in preference order: + - **(recommend MVP) A lightweight local classifier / lexicon** over the redacted text — + e.g. a fine-tuned or off-the-shelf sentiment model (`distilbert-sst2`-class) run in the + same batch job as embedding. CPU-cheap, no per-record API cost, keeps PII local. + Validate its output against the tier-1 explicit signals to pick a threshold. + - **(fallback) LLM classification** only if the local classifier's accuracy against the + explicit-signal validation set is inadequate. Even then, batch it and cap it — do not + make it a per-record online cost. + - **Semantic/embedding mapping** (the task title's phrasing): the embeddings already + computed (§B) can be nearest-neighbour-mapped to the explicit-signal-labeled examples + as a zero-extra-model-cost sentiment estimate. This is the most economical option and + reuses the one embedding — evaluate it against the local-classifier option on a sample. + +**Decision handed to implementation with a concrete evaluation:** run all three (explicit ++ embedding-kNN, explicit + local classifier, explicit + LLM) on a labeled Zendesk sample, +pick by accuracy-vs-cost. Default assumption: **explicit signals + embedding-kNN** wins on +cost and is "good enough" for trend-level sentiment; upgrade only if the accuracy gap is +material. `dim_sentiment` and the fact column are unaffected by which wins. + +--- + +## F. What is explicitly deferred (non-blocking for spec/MVP) + +- Embedding model final pick + GPU/CPU throughput at full 1.18M scale (MVP proves it at 198K). +- Dedicated vector store / online serving (Iceberg ARRAY suffices for batch). +- Aspect-based sentiment; multi-lingual handling. +- Semantic-summary-before-embedding and hierarchical truncation from prototype #10793 — + test as hypotheses on a sample, do not inherit (RFC Open Questions). +- Cross-source `afact_feedback_cluster_daily` tuning (Phase 2). diff --git a/docs/design/feedback_zendesk_mvp_spec.md b/docs/design/feedback_zendesk_mvp_spec.md new file mode 100644 index 000000000..85d2d0359 --- /dev/null +++ b/docs/design/feedback_zendesk_mvp_spec.md @@ -0,0 +1,243 @@ +# Feedback Aggregation — Zendesk MVP Implementation Spec + +Status: **spec** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Companion to [`feedback_dimensional_model.md`](./feedback_dimensional_model.md) +and [`feedback_ml_approach.md`](./feedback_ml_approach.md) + +Concrete, build-ready spec for the first slice: **Zendesk tickets → `tfact_feedback`**, +serving support + engineering. Grounded in the actual repo models/columns. Everything here +is Phase-1 MVP scope (design §9); forum/tutor/ORA and the data-bus migration are later. + +Convention baseline: mirrors `tfact_discussion_events` (the existing multi-source fact) — +same timestamp macros, same FK-by-lookup-join pattern, same `_dim__models.yml` contract +style. Divergence from precedent is called out explicitly where intentional. + +--- + +## 1. dbt model DAG (MVP) + +``` +raw__thirdparty__zendesk_support__tickets (existing Airbyte raw, in _zendesk__sources.yml) + → stg__zendesk__ticket (existing) + → int__zendesk__ticket (existing, one row per ticket — grain source) + │ + ├─ stg__zendesk__user (existing — for requester email → dim_user) + │ + ▼ + int__feedback__zendesk (NEW — conform ticket to the common event contract §5 of design doc) + ▼ + int__feedback__unioned (NEW — UNION of all sources; MVP = zendesk only; + PII redaction §7) + ▼ + tfact_feedback (NEW — resolve conformed FKs, generate feedback_pk) + + dim_feedback_source (NEW — seed rows, static/near-static) + dim_feedback_category (NEW — seeded from ticket_tags + group_name; LLM-labeled later) + dim_sentiment (NEW — seeded static buckets) +``` + +The two-hop `int__feedback__zendesk → int__feedback__unioned` looks redundant at MVP (one +source) but is deliberate: `__unioned` is where new sources plug in without touching the +fact, and where redaction happens once for all sources. Keeping it from day one makes +Phase 2 a pure additive change. + +--- + +## 2. `int__feedback__zendesk` (NEW) — conform Zendesk to the common contract + +Reads `int__zendesk__ticket` (grain: one row per ticket, `ticket_id` unique+not_null), +left-joined to `stg__zendesk__ticket` (for `ticket_requester_user_id`) → `stg__zendesk__user` +(for `user_email`, needed because `int__zendesk__ticket` only carries `ticket_requester` +as a *name*, not an id/email — confirmed in source inventory). + +Output columns = the common feedback event contract (design §5), source-typed: + +| Contract field | Zendesk expression | +|---|---| +| `source_slug` | literal `'zendesk'` | +| `occurred_at` | `ticket_created_at` (ISO8601) | +| `subject_user_ref` | `user_email` from `stg__zendesk__user` via `ticket_requester_user_id` (last-resort identity path, see §4) | +| `courserun_readable_id` | `null` (Zendesk is not course-scoped) | +| `platform` | `null` | +| `conversation_ref` | `ticket_id` (thread/ticket id) | +| `source_record_ref` | `ticket_id` (**idempotency + business key**, §6 of design) | +| `title` | `ticket_subject` | +| `text` | `ticket_description` (= first comment body) | +| `source_metadata` | struct/JSON: `ticket_tags`, `ticket_status`, `ticket_priority`, `ticket_source_channel`, `ticket_satisfaction_rating_score`, `ticket_satisfaction_rating_comment`, `group_name`, `organization_name`, `ticket_api_url` | + +Notes: +- **`ticket_description` = first comment only** (confirmed). That is the correct MVP grain + per design §1 (Zendesk row = first comment). Full-thread text via + `int__zendesk__ticket_comment` is a Phase-2 option, not MVP. +- Carry `ticket_api_url` through as the `source_url` deep-link. +- Do **not** redact here — redaction is centralized in `int__feedback__unioned` (§3). + +--- + +## 3. `int__feedback__unioned` (NEW) — union + redact + +- **Union** all per-source `int__feedback__` models into the single common shape. + MVP: just `int__feedback__zendesk`. The model is written as an explicit `union all` of + CTEs (mirrors `tfact_discussion_events`' per-source CTE style) so adding forum/tutor is a + new CTE + one `union all` line. +- **PII redaction (design §7, mandatory pre-embed):** apply the Presidio-based masking to + `title` and `text` here, producing `title_redacted` / `text_redacted`. Raw `title`/`text` + do **not** propagate past this model — only redacted text flows to `tfact_feedback` and + the embedding store. Raw text remains available upstream in `stg`/`int__zendesk` under + existing PII classification + Lakekeeper/Cedar authz. + - Implementation note: Presidio is Python, not SQL. Two viable placements — (a) a Python + Dagster asset that materializes the redacted column between `int__feedback__unioned` + and the fact, or (b) a dbt Python model if the warehouse adapter supports it. **Recommend + (a)** for MVP: the same batch asset that embeds also redacts, single Python surface, + and it keeps the dbt layer pure-SQL (repo convention). Revisit if a SQL-native masking + macro is preferred. This is the one place the spec's dbt DAG and the Dagster asset graph + interleave — see §7. +- Carry `feedback_text_chars = length(text)` (pre-redaction length metric, design §2) for + sizing/analytics; computing length before masking is fine (no PII in an integer). + +--- + +## 4. `tfact_feedback` (NEW) — the fact + +Mirrors `tfact_discussion_events` conventions. Reads `int__feedback__unioned`. + +**Grain:** one row per atomic feedback utterance. `feedback_pk` unique. + +**Surrogate + FK resolution:** +```sql +-- surrogate business key (DIVERGES from precedent — see note) +{{ dbt_utils.generate_surrogate_key(['source_slug', 'source_record_ref']) }} as feedback_pk + +-- conformed FK: source +{{ dbt_utils.generate_surrogate_key(['source_slug']) }} as feedback_source_fk + +-- conformed FK: user (nullable). Zendesk = last-resort email path. +users.user_pk as user_fk +... +left join dim_user as users + on lower(unioned.subject_user_ref) = users.email -- dim_user.user_pk = surrogate_key(lower(email)) + +-- conformed FKs not populated for Zendesk (nullable, correct): +-- courserun_fk, platform_fk -> null at MVP +-- organization_fk -> resolve from source_metadata.organization_name if/when a +-- dim_organization join key exists; null-tolerant otherwise + +-- late-arriving, null at insert, updated by ML asset (design §4a/§4b): +cast(null as varchar) as category_fk +cast(null as varchar) as sentiment_fk + +-- time +{{ iso8601_to_time_key('occurred_at') }} as time_fk +{{ iso8601_to_date_key('occurred_at') }} as date_fk +``` + +**Identity resolution — the highest-risk join (design §3, RFC Consequences).** Zendesk has +no openedx user id, so `user_fk` resolves via **email → `dim_user.email`** (which is how +`dim_user.user_pk` itself is generated: `generate_surrogate_key(['lower(email)'])`). This is +the last-resort path and shares the failure class of the open p0 `dim_user` NULL-email +identity-collapse bug (`tk-re-derive-identity-conformed-dimension-joins-pos-b7ca16`). Guard: +- Never key `feedback_pk` off the resolved `user_fk` (it keys off `source_record_ref`), so a + bad identity join can never collapse or duplicate the fact grain. +- `user_fk` stays **nullable**; an unresolved requester email = null `user_fk`, not a wrong + join. Do not coalesce to a sentinel. +- Re-run `tk-...-b7ca16` before enabling any cross-source identity rollups on this fact. + +**Divergence from precedent (intentional):** `tfact_chatbot_events`/`tfact_discussion_events` +mint no `*_pk` and rely on a model-level `expect_compound_columns_to_be_unique` test. This +fact mints an explicit `feedback_pk` from the stable source business key because (1) the +migration strategy (design §6) requires the same PK to regenerate identically across the +interim→data-bus source swap, and (2) `category_fk`/`sentiment_fk` are late-arriving updates +that need a stable row key to target. We keep the compound-uniqueness test *as well* (§8). + +**Output columns** (fact): `feedback_pk`, `feedback_source_fk`, `user_fk`, `courserun_fk`, +`platform_fk`, `organization_fk`, `category_fk`, `sentiment_fk`, `time_fk`, `date_fk`, +`conversation_id` (=`conversation_ref`), `source_record_id` (=`source_record_ref`), +`source_url`, `feedback_title` (redacted), `feedback_text` (redacted), `feedback_text_chars`, +`embedding_id` (nullable), `source_status`, `source_priority`, `source_tags`, +`source_channel`, `csat_score`, `feedback_occurred_at`, `feedback_ingested_at`. + +--- + +## 5. New dimensions (MVP) + +### `dim_feedback_source` — static seed +Rows for MVP: one, `zendesk`. Columns per design §4c +(`feedback_source_pk = generate_surrogate_key(['source_slug'])`, `source_slug`, `source_name`, +`source_medium='support_ticket'`, `source_audience_scope='operational'`, `is_course_scoped=false`). +Implement as a dbt seed (`seeds/`) or a small `select ... union all` model — recommend a +seed CSV since the set is tiny and hand-curated. + +### `dim_sentiment` — static seed +Rows: `positive`, `neutral`, `negative` (design §4b), `sentiment_pk = generate_surrogate_key(['sentiment_slug'])`, +`polarity_score_bucket`. dbt seed CSV. + +### `dim_feedback_category` — seeded, then ML-curated +- **MVP seed (no ML):** distinct `ticket_tags` (~2,354) + `group_name` from + `int__zendesk__ticket`, materialized as `category_source='seed'`, + `category_status='proposed'`, `category_slug = generate_surrogate_key([slugified tag])`. + A dbt model that unnests `ticket_tags` and selects distinct, plus group names. +- **ML curation (later, per `feedback_ml_approach.md` §D):** LLM-labeled clusters upsert + `category_source='llm_discovered'` rows; humans flip `category_status` to `approved`. +- SCD-lite: relabel changes `category_label`, never `category_slug`. + +--- + +## 6. Sentiment & category assignment at MVP + +- **Sentiment (`sentiment_fk`):** MVP can populate a *coarse* sentiment immediately from the + explicit signal with **no model**: map `ticket_satisfaction_rating_score` + (`'good'`→positive, `'bad'`→negative, `'offered'`/null→neutral/unknown) → `dim_sentiment`. + The model-based sentiment (`feedback_ml_approach.md` §E) upgrades the null/`offered` rows + later. This gives a working sentiment facet on day one for the rated subset. +- **Category (`category_fk`):** MVP can assign the tag-seed category by mapping a ticket's + dominant `ticket_tag` → its seed `category_slug`. Cluster-based reassignment comes with the + ML asset. Unassigned = null (queryable). + +Both are late-arriving updates, so the fact builds and is useful before the ML asset exists. + +--- + +## 7. Dagster asset (MVP) — SEE `feedback_dagster_asset_spec.md` + +The scheduled batch asset (pull → redact → embed → cluster → LLM-label → write +category/sentiment back) is specified separately once the orchestration layout is confirmed. +The dbt models above are independently buildable and testable *without* the ML asset — the +ML asset only fills `embedding_id`, `category_fk`, `sentiment_fk`, and the `feedback_embeddings` +sidecar. This ordering lets the fact ship first. + +--- + +## 8. Tests / contract (`_dim__models.yml` entries) + +Mirror the `tfact_discussion_events` yml style: +- Per-column `not_null` on: `feedback_pk`, `feedback_source_fk`, `source_slug` equivalent, + `source_record_id`, `feedback_occurred_at`, `time_fk`, `date_fk`. +- `unique` on `feedback_pk`. +- Nullable (description-only, no not_null): `user_fk`, `courserun_fk`, `platform_fk`, + `organization_fk`, `category_fk`, `sentiment_fk`, `embedding_id`. +- Model-level `dbt_expectations.expect_compound_columns_to_be_unique` on + `['source_slug', 'source_record_id']` (belt-and-suspenders alongside the `feedback_pk` + unique test — matches the precedent's compound-uniqueness convention). +- `relationships` tests from each `*_fk` to its dim PK (richer-contract style, as + `dim_course_run` does). +- New dims get their own entries; `dim_feedback_category` gets a `unique` on `category_slug`. + +--- + +## 9. Build & verify path (local) + +Per repo convention (`ol-dbt` CLI, DuckDB-over-Iceberg local): after writing models, run +`local register` + a targeted `dbt build --select +tfact_feedback` to validate the fact and +its upstreams compile and pass tests against live Iceberg data. Expected MVP volume ~198K +rows, single batch (design §9). Validate: `feedback_pk` uniqueness, null-`user_fk` rate +(sanity-check identity resolution isn't silently collapsing), and row count vs. +`int__zendesk__ticket`. + +--- + +## 10. Scope boundary (what this MVP does NOT do) + +- No forum/tutor/ORA sources (Phase 2 — additive CTEs in `int__feedback__unioned`). +- No `afact_feedback_cluster_daily` aggregate (Phase 2). +- No data-bus/analytics-api ingress (Phase 3, gated on the write path — RFC Open Questions). +- No embedding/clustering *required* for the fact to be useful (ML asset is additive). +- No cross-source identity rollups until `tk-...-b7ca16` is re-derived. From e322eaf15ec37eaa4479eea6c4f6f123181a72a6 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 08:05:10 -0400 Subject: [PATCH 2/7] =?UTF-8?q?docs(feedback):=20ADR=20=E2=80=94=20push=20?= =?UTF-8?q?embedding/AI=20compute=20into=20Starburst=20Galaxy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Evaluate in-engine AI functions and a DataFrame AI framework against the original local-torch embedding plan. Decisive repo fact: production runs Trino on Starburst Galaxy (StarRocks is not deployed), whose in-SQL AI functions (generate_embedding → Iceberg ARRAY(DOUBLE), public preview) can embed inside the engine we already run, on AWS Bedrock in-account. New default: Starburst AI generate_embedding as a trino_only dbt model — eliminates the net-new torch/sentence-transformers service and removes the third-party-egress objection (Bedrock-in-account over redacted text). Only clustering + LLM cluster-labeling remain a (now lightweight) Python asset, since neither Trino nor DuckDB does density clustering and we want HDBSCAN's noise class. Fenic and local self-hosted embeddings kept as fallbacks; StarRocks vector index folded into the Phase-3 data-bus note. Adds adr_embedding_compute_strategy.md; revises ml_approach §B, dagster_asset_spec header, and the README index/decisions accordingly. Schema, contract, and business keys unchanged (compute location was always meant to be swappable — vectors in a sidecar, category/sentiment late-arriving). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RJXxq9oEabCbJiXFzSTqkY --- docs/design/README_feedback_aggregation.md | 9 +- docs/design/adr_embedding_compute_strategy.md | 134 ++++++++++++++++++ docs/design/feedback_dagster_asset_spec.md | 11 ++ docs/design/feedback_ml_approach.md | 24 +++- 4 files changed, 170 insertions(+), 8 deletions(-) create mode 100644 docs/design/adr_embedding_compute_strategy.md diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md index c07e74665..76bcacf6d 100644 --- a/docs/design/README_feedback_aggregation.md +++ b/docs/design/README_feedback_aggregation.md @@ -19,6 +19,7 @@ signals for four audiences (support, engineering, instructors, leadership). | [`feedback_ml_approach.md`](./feedback_ml_approach.md) | Embedding (local, PII-safe), clustering (UMAP+HDBSCAN), category discovery (seed + LLM-label), sentiment (explicit + kNN/classifier) | `...clustering-...a1d7d6`, `...category-...550aba`, `...sentiment-...92988e` | | [`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md) | Batch ML Dagster asset cloned from `student_risk_probability`; Vault-backed LLM resource; net-new deps; scheduling | (ML pipeline orchestration) | | [`feedback_consumption_ux_spec.md`](./feedback_consumption_ux_spec.md) | Audiences × altitude, Superset-first surfaces, per-persona actions, access control | `...ui-ux-...476d23` | +| [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md) | ADR: where embedding/AI inference runs — Starburst Galaxy AI functions (Bedrock, in-SQL) vs. Fenic vs. local vs. StarRocks | (revises embedding default) | ## Key decisions (spec phase) @@ -33,8 +34,12 @@ signals for four audiences (support, engineering, instructors, leadership). tag-seeded categories + CSAT-derived sentiment; embeddings/clustering fill `category_fk`/ `sentiment_fk`/`embedding_id` later. Embeddings persisted **once** (the one adopted lesson from prototype #10793). -4. **Local, PII-safe embeddings** by default (learner-PII posture); redaction (Presidio) - mandatory pre-embed; raw text stays upstream under existing Lakekeeper/Cedar authz. +4. **Embedding compute pushed into the engine we already run** (revised 2026-07-10, ADR): + production is Trino on **Starburst Galaxy**, so `generate_embedding` (Bedrock, in-account, + public preview) generates vectors in SQL/dbt into the Iceberg sidecar — eliminating the + net-new torch service. Redaction (Presidio) stays mandatory pre-embed. Local self-hosted + embeddings and Fenic are documented fallbacks; StarRocks vector search is a Phase-3 note + (not deployed today). 5. **Superset-first consumption**; support + engineering are the MVP-served audiences (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). diff --git a/docs/design/adr_embedding_compute_strategy.md b/docs/design/adr_embedding_compute_strategy.md new file mode 100644 index 000000000..b502f826e --- /dev/null +++ b/docs/design/adr_embedding_compute_strategy.md @@ -0,0 +1,134 @@ +# ADR: Where feedback embedding / AI inference runs + +Status: **proposed** · Project: `wp-feedback-aggregation-clustering-system-2e9750` +Date: 2026-07-10 · Amends [`feedback_ml_approach.md`](./feedback_ml_approach.md) §B/§E and +[`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md). + +## Context + +The original ML spec (`feedback_ml_approach.md` §B) proposed a **net-new Dagster code +location** running `sentence-transformers`/`torch` to embed feedback locally, chosen for the +learner-PII posture (avoid shipping ~1.18M utterances to a third-party API). The Dagster +explore flagged this as the heaviest part of the whole system: a large image (torch + spaCy), +a first-of-its-kind ML service in the repo, and all-net-new deps. + +A prompt to evaluate **in-engine AI functions** (Trino/Starburst, StarRocks) and a +**DataFrame-based AI framework** (Fenic) against that approach surfaced a decisive fact from +the repo: + +- **Production runs Trino on Starburst Galaxy** (`*.trino.galaxy.starburst.io`; + `src/ol_dbt/profiles.yml` production/qa targets `type: trino`). Local dev is DuckDB. +- **StarRocks is NOT deployed** — it exists only as `starrocks__` dispatch branches in + `macros/cross_db_functions.sql` and "future" language in `DBT_DIALECT_COMPATIBILITY.md`. +- There is **zero existing use** of engine-native AI/embedding/vector SQL functions. + +This means the engine we already run has AI functions available, and the vector-DB engine we +don't run is the only one the "StarRocks vector index" idea depended on. + +## The three candidates, disambiguated + +They are **not** the same kind of thing — the original prompt ("use a dataframe for the AI +functions in Trino/Starburst or StarRocks") blends them; they separate cleanly: + +| Candidate | What it is | Runs where | Generates embeddings? | Vector search? | +|---|---|---|---|---| +| **Starburst Galaxy AI functions** | In-SQL `starburst.ai.generate_embedding(text, model)` → `ARRAY(DOUBLE)`, plus `analyze_sentiment`, `classify`, `mask`, `prompt` | **inside our production Trino engine** | **yes** (Bedrock/OpenAI) | via generated vectors + similarity | +| **Fenic** (typedef.ai) | PySpark-style **client-side** DataFrame lib with semantic operators (`embed`, `with_cluster_labels` K-means, `analyze_sentiment`, `classify`, `semantic.join`) | a Python process (a Dagster asset) | **yes** (OpenAI/Google/Cohere APIs — cloud only) | K-means clustering built in | +| **StarRocks vector index** | HNSW/IVFPQ index + `approx_cosine_similarity` | inside StarRocks — **not deployed** | **no** (store/search only) | yes (ANN) | + +Key correction to the original framing: **Fenic does not "use" Trino/StarRocks AI functions.** +It has its *own* embedding/semantic operators and runs client-side; it is an alternative to a +hand-rolled Python pipeline, not a way to invoke engine AI functions. StarRocks *stores & +searches* vectors but does **not** generate them. Only Starburst pushes embedding generation +into the engine we already run. + +## Decision + +**Primary: push embedding + model-based sentiment into SQL/dbt via Starburst Galaxy AI +functions, on AWS Bedrock models, over Presidio-redacted text. Keep only clustering + +LLM cluster-labeling as a (now lightweight) Python Dagster asset.** + +Revised pipeline (replaces `feedback_ml_approach.md` §A for the embed/sentiment stages): + +``` +int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL] + → feedback_embeddings : generate_embedding() → ARRAY(DOUBLE) in Iceberg [dbt, trino_only] + → sentiment_fk : analyze_sentiment() (+ explicit CSAT seed §E) [dbt, trino_only] + → feedback_clusters : read vectors from Iceberg → UMAP+HDBSCAN [Dagster py asset] + → dim_feedback_category : LLM-label clusters (Bedrock via prompt()) [Dagster py asset] +``` + +Rationale: +1. **Eliminates the heaviest infra.** No torch/sentence-transformers, no embedding model in a + Docker image. Embedding + sentiment become `trino_only`-tagged dbt models. The Python + surface shrinks to *just* clustering + labeling — which read pre-computed vectors, so the + asset stays tiny (sklearn/UMAP, CPU) and still clones `student_risk_probability`. +2. **Vectors land where dbt already lives.** `generate_embeddings` procedure (or + `generate_embedding()` in an INSERT) writes straight into the `feedback_embeddings` Iceberg + `ARRAY(DOUBLE)` column — no new store, no data movement. +3. **PII posture improves, doesn't regress.** Starburst AI runs against **AWS Bedrock**, which + can be in our own AWS account/region (we already run on AWS: Glue/S3/EKS). Bedrock-in-account + over Presidio-redacted text removes the "third-party egress" objection that drove the + original local-only recommendation. (Presidio stays the deterministic redaction guarantee; + `ai.mask` is an LLM-based masker to *evaluate as an add-on*, not to trust for compliance.) +4. **Already on the engine.** No new service to run or operate — it's a feature of the Trino + cluster we already pay for. + +**Clustering stays Python** because neither Trino nor DuckDB does density clustering, and we +specifically want **HDBSCAN's noise class** to separate one-off complaints from systemic +signal (`feedback_ml_approach.md` §C) — Fenic's `with_cluster_labels` is **K-means only** (no +noise class), so it does not satisfy that requirement. + +## Consequences / caveats (must be resolved before committing) + +1. **Gating question — is the AI-workflows feature enabled on our Galaxy plan?** + `generate_embedding` is **public preview** in Starburst Galaxy. Confirm it's available on + our subscription and that we accept depending on a preview feature for a production pipeline + (API-stability risk). This is the single decision that makes-or-breaks the primary option. +2. **Bedrock model access + config.** Requires configuring an embedding model (e.g. + `bedrock_titan`) and a language model in Galaxy AI Model Access Management, and enabling + Bedrock in our AWS account. New platform setup, but far lighter than a torch service. +3. **DuckDB local-dev / CI incompatibility.** AI functions are Trino-only; the embedding and + `analyze_sentiment` models must carry `tags: ['trino_only']` (existing repo pattern) and + will **not** run under the DuckDB `dev_local` target or the DuckDB-based docs CI. Embedding + is not something we'd want running on every local build anyway (cost), so gating it is + acceptable — but you lose full local reproducibility of those two models. +4. **Cost shifts from one-time-local to per-invocation-provider.** Original: ~$2–16 one-time + local compute. Now: Bedrock embedding cost per record (Titan text embeddings are cheap — + order low-tens-of-dollars for 1.18M short redacted utterances — but it's a provider line + item, and re-embedding must still be avoided via persist-once + `model_version`). Do a + quick Bedrock-pricing sanity check at spec-approval time. +5. **`analyze_sentiment`/`classify` are LLM calls per row.** Fine at MVP volume (198K, batch), + but confirm the batch cost and that per-row LLM sentiment beats the cheaper + explicit-CSAT-seed + embedding-kNN option (`feedback_ml_approach.md` §E) — it may not, in + which case keep sentiment on the kNN path and use Starburst AI only for embedding. + +## Alternatives (kept, in preference order) + +- **Fenic (fallback if the Galaxy AI preview is unavailable/undesired).** Gives + batching/caching/cost-accounting/row-level lineage for the Python path and does embed + + sentiment + K-means in one DataFrame pipeline reading Parquet/S3. Downsides vs. the primary: + still client-side (no engine push-down), **cloud-API egress only** (no self-hosted / no + in-account Bedrock embedding path documented), K-means clustering (no HDBSCAN noise class), + and a new framework dependency. Reasonable middle ground; less aligned than SQL push-down + given we're already on Galaxy. (License: OSS public repo — confirm Apache-2.0 before adopting.) +- **Local `sentence-transformers` in Dagster (original §B).** Now the *last* resort: only if + both the Starburst AI feature and cloud/Bedrock egress are ruled out. Zero per-record + provider cost and no egress, but the heaviest image/infra and the most net-new code. +- **StarRocks vector index — deferred to Phase 3.** Not applicable now (not deployed). When/if + the data bus lands on StarRocks, migrate the `feedback_embeddings` sidecar from Iceberg + `ARRAY(DOUBLE)` brute-force to a StarRocks HNSW index for ANN serving (similar-feedback / RAG + at query time). StarRocks still won't *generate* embeddings — those keep coming from Starburst + AI (or a UDF). Fold into the `afact`/serving work, not the MVP. + +## Net change to the spec + +- `feedback_ml_approach.md` §B default flips: **Starburst Galaxy AI `generate_embedding` + (Bedrock, in-account, on redacted text)** becomes the recommended embedding path; + local self-hosted drops to fallback. +- `feedback_dagster_asset_spec.md`: the `feedback_embeddings` and `feedback_sentiment` stages + move from Python assets to `trino_only` dbt models; the Dagster asset shrinks to + `feedback_clusters` + `feedback_category_proposals` (read vectors, UMAP+HDBSCAN, LLM-label). +- No change to the fact, the dims, the contract, or the business keys — this ADR only changes + *where the vector/sentiment compute runs*, which the schema was deliberately built to be + agnostic about (embeddings live in a sidecar; category/sentiment are late-arriving FKs). diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md index 6eca2797f..d67713f5d 100644 --- a/docs/design/feedback_dagster_asset_spec.md +++ b/docs/design/feedback_dagster_asset_spec.md @@ -9,6 +9,17 @@ LLM-proposed categories, and sentiment. Grounded in the existing repo orchestrat patterns. **The fact ships without this asset**; this is purely additive (fills `embedding_id`, `category_fk`, `sentiment_fk`, and the `feedback_embeddings` sidecar). +> **REVISED 2026-07-10 — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> Because production runs **Starburst Galaxy** (Trino), the **embedding** and **model-based +> sentiment** stages move OUT of this Python asset and INTO `trino_only` dbt models using +> Starburst AI functions (`generate_embedding` → Iceberg `ARRAY(DOUBLE)`; `analyze_sentiment`). +> This asset then shrinks to **only what SQL cannot do: clustering + LLM cluster-labeling** — +> read pre-computed vectors from the Iceberg sidecar → UMAP+HDBSCAN → LLM-label clusters. No +> embedding model in the image (no torch/sentence-transformers), so it stays a small +> sklearn/UMAP CPU asset still cloned from `student_risk_probability`. §2–§6 below describe the +> full-fat Python version; treat the embed/sentiment assets there as the fallback used only if +> the Starburst AI preview is not adopted (ADR "Alternatives"). + --- ## 1. Template: clone `student_risk_probability` diff --git a/docs/design/feedback_ml_approach.md b/docs/design/feedback_ml_approach.md index 1323cff01..040e48c7d 100644 --- a/docs/design/feedback_ml_approach.md +++ b/docs/design/feedback_ml_approach.md @@ -38,13 +38,25 @@ so a re-run never duplicates and never mutates the fact grain. Embedding is comp **Decision: one shared embedding, computed once, stored in the `feedback_embeddings` sidecar.** Both clustering and (semantic) sentiment consume it — do not embed twice. -- **Model choice — recommend an open/self-hostable model** (e.g. `BAAI/bge-small-en-v1.5` - or `sentence-transformers/all-MiniLM-L6-v2`, 384-dim) over a hosted API, for the - **learner-PII posture**: even post-redaction, sending ~1.18M feedback utterances to a - third-party embedding API is an avoidable data-egress and cost exposure. A local +> **REVISED 2026-07-10 — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> Production runs Trino on **Starburst Galaxy**, whose in-SQL AI functions +> (`starburst.ai.generate_embedding` → `ARRAY(DOUBLE)`, public preview) can generate +> embeddings **inside the engine we already run**, on **AWS Bedrock in our own AWS account**, +> writing vectors straight into the Iceberg sidecar. That both eliminates the net-new +> torch/sentence-transformers service *and* removes the third-party-egress objection below +> (Bedrock-in-account over Presidio-redacted text). **New default: Starburst AI +> `generate_embedding` as a `trino_only` dbt model.** The local option below drops to a +> last-resort fallback. StarRocks vector functions are irrelevant for now (not deployed) — see +> the ADR for the Phase-3 note. The rest of §B (persist-once, `model_version`, Iceberg ARRAY +> storage, redaction-upstream) is unchanged and applies regardless of who computes the vector. + +- **Model choice (fallback path, if the Starburst AI preview is unavailable/undesired) — + an open/self-hostable model** (e.g. `BAAI/bge-small-en-v1.5` or + `sentence-transformers/all-MiniLM-L6-v2`, 384-dim): even post-redaction, sending ~1.18M + feedback utterances to a *third-party* embedding API is avoidable egress — but note this + concern does **not** apply to Bedrock-in-account via Starburst AI (the new default). A local sentence-transformers model runs the full corpus in a single batch job on CPU/GPU for - effectively $0 marginal cost. Keep an OpenAI `text-embedding-3-small` adapter behind the - same resource interface as a fallback/benchmark, but default to local. + effectively $0 marginal provider cost, at the price of the heaviest image/infra. - Open question deferred to implementation: GPU vs CPU throughput at 1.18M rows. At MVP scale (198K) CPU is fine (single-digit minutes to low hours). - **Redaction is upstream and mandatory** (design §7): embeddings are computed on the From 46fc55a62623244c2cbaebc34c5efed4a74e496e Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 08:29:46 -0400 Subject: [PATCH 3/7] =?UTF-8?q?docs(feedback):=20ADR=20rev.2=20=E2=80=94?= =?UTF-8?q?=20keep=20AI=20compute=20engine-portable=20(Trino=E2=86=92StarR?= =?UTF-8?q?ocks)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strategic direction is to retire Trino for StarRocks (transforms + data bus + OLAP serving), so no Galaxy-only functionality may sit on the critical path. This withdraws rev.1's Starburst generate_embedding recommendation: it is a Galaxy-proprietary SQL function in exactly the layer being migrated, and StarRocks has no embedding-generation equivalent to migrate it to. New decision: keep all AI/embedding compute engine-external (Dagster/Python) reading/writing open Iceberg, and generate embeddings by calling AWS Bedrock (amazon.titan-embed-text-v2:0) DIRECTLY via boto3 — same in-account/PII-safe Bedrock substrate Starburst's function merely wraps, minus the lock-in and minus torch. Vectors land in an Iceberg ARRAY sidecar that StarRocks later reads to build an HNSW index (a load, not a re-embed), making StarRocks ANN the intended serving tier. Sentiment stays the engine-agnostic CSAT-seed + embedding-kNN path (no analyze_sentiment); clustering stays Python HDBSCAN. dbt models stay portable (no trino_only AI models). Fenic (cloud-egress, K-means) and local sentence-transformers kept as fallbacks. Schema/contract/business-keys unchanged — compute location was always meant to be swappable. --- docs/design/README_feedback_aggregation.md | 15 +- docs/design/adr_embedding_compute_strategy.md | 216 +++++++++--------- docs/design/feedback_dagster_asset_spec.md | 21 +- docs/design/feedback_ml_approach.md | 43 ++-- 4 files changed, 153 insertions(+), 142 deletions(-) diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md index 76bcacf6d..2cc9fd9e8 100644 --- a/docs/design/README_feedback_aggregation.md +++ b/docs/design/README_feedback_aggregation.md @@ -34,12 +34,15 @@ signals for four audiences (support, engineering, instructors, leadership). tag-seeded categories + CSAT-derived sentiment; embeddings/clustering fill `category_fk`/ `sentiment_fk`/`embedding_id` later. Embeddings persisted **once** (the one adopted lesson from prototype #10793). -4. **Embedding compute pushed into the engine we already run** (revised 2026-07-10, ADR): - production is Trino on **Starburst Galaxy**, so `generate_embedding` (Bedrock, in-account, - public preview) generates vectors in SQL/dbt into the Iceberg sidecar — eliminating the - net-new torch service. Redaction (Presidio) stays mandatory pre-embed. Local self-hosted - embeddings and Fenic are documented fallbacks; StarRocks vector search is a Phase-3 note - (not deployed today). +4. **Engine-portable AI compute** (revised 2026-07-10 rev. 2, ADR): because the strategic + direction is to **retire Trino for StarRocks**, no Galaxy-only functionality sits on the + critical path. Embedding stays engine-external in a Dagster asset calling **AWS Bedrock + `amazon.titan-embed-text-v2:0` directly via boto3** (in-account → PII-safe over redacted + text; no torch; indifferent to Trino vs. StarRocks). Vectors land in an open Iceberg + `ARRAY` sidecar; **StarRocks HNSW is the intended vector-serving tier**, reached by + loading those vectors (no re-embed). Redaction (Presidio) mandatory pre-embed. Fenic and + local `sentence-transformers` are fallbacks; Starburst `generate_embedding` is **rejected** + for engine lock-in. 5. **Superset-first consumption**; support + engineering are the MVP-served audiences (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). diff --git a/docs/design/adr_embedding_compute_strategy.md b/docs/design/adr_embedding_compute_strategy.md index b502f826e..ee3dbbba1 100644 --- a/docs/design/adr_embedding_compute_strategy.md +++ b/docs/design/adr_embedding_compute_strategy.md @@ -1,134 +1,140 @@ # ADR: Where feedback embedding / AI inference runs Status: **proposed** · Project: `wp-feedback-aggregation-clustering-system-2e9750` -Date: 2026-07-10 · Amends [`feedback_ml_approach.md`](./feedback_ml_approach.md) §B/§E and +Date: 2026-07-10 (rev. 2 — engine-portability constraint) · Amends +[`feedback_ml_approach.md`](./feedback_ml_approach.md) §B/§E and [`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md). ## Context -The original ML spec (`feedback_ml_approach.md` §B) proposed a **net-new Dagster code -location** running `sentence-transformers`/`torch` to embed feedback locally, chosen for the -learner-PII posture (avoid shipping ~1.18M utterances to a third-party API). The Dagster -explore flagged this as the heaviest part of the whole system: a large image (torch + spaCy), -a first-of-its-kind ML service in the repo, and all-net-new deps. +The original ML spec (`feedback_ml_approach.md` §B) proposed a net-new Dagster code location +running `sentence-transformers`/`torch` to embed feedback locally, for the learner-PII posture. +The Dagster explore flagged this as the heaviest part of the system (large image, first ML +service, all-new deps). -A prompt to evaluate **in-engine AI functions** (Trino/Starburst, StarRocks) and a -**DataFrame-based AI framework** (Fenic) against that approach surfaced a decisive fact from -the repo: +Two facts then reshaped the decision: -- **Production runs Trino on Starburst Galaxy** (`*.trino.galaxy.starburst.io`; - `src/ol_dbt/profiles.yml` production/qa targets `type: trino`). Local dev is DuckDB. -- **StarRocks is NOT deployed** — it exists only as `starrocks__` dispatch branches in - `macros/cross_db_functions.sql` and "future" language in `DBT_DIALECT_COMPATIBILITY.md`. -- There is **zero existing use** of engine-native AI/embedding/vector SQL functions. +1. **Production runs Trino on Starburst Galaxy** (`src/ol_dbt/profiles.yml`); local dev is + DuckDB; **StarRocks is not deployed** (only `starrocks__` dispatch branches in + `macros/cross_db_functions.sql`). +2. **Strategic direction (owner, 2026-07-10): retire Trino in favour of StarRocks** as the + single engine for warehouse/Iceberg transformations, the "data bus", and OLAP serving. + **Therefore: no Galaxy-only (Starburst-proprietary) functionality may sit on the critical + path** — anything we build on the SQL engine must survive the Trino→StarRocks migration. -This means the engine we already run has AI functions available, and the vector-DB engine we -don't run is the only one the "StarRocks vector index" idea depended on. +Rev. 1 of this ADR recommended pushing embedding into SQL via Starburst Galaxy's +`generate_embedding`. **Fact 2 vetoes that**: it is a Galaxy-proprietary SQL function living in +exactly the layer being migrated to StarRocks, and StarRocks has **no** embedding-generation +function to migrate it to (StarRocks vector support is *store + ANN search* only — +`cosine_similarity`, HNSW/IVFPQ indexes — not generation). Adopting it would guarantee a +rewrite. This revision removes it from the critical path. ## The three candidates, disambiguated -They are **not** the same kind of thing — the original prompt ("use a dataframe for the AI -functions in Trino/Starburst or StarRocks") blends them; they separate cleanly: - -| Candidate | What it is | Runs where | Generates embeddings? | Vector search? | +| Candidate | What it is | Runs where | Generates embeddings? | Engine-portable (Trino→StarRocks)? | |---|---|---|---|---| -| **Starburst Galaxy AI functions** | In-SQL `starburst.ai.generate_embedding(text, model)` → `ARRAY(DOUBLE)`, plus `analyze_sentiment`, `classify`, `mask`, `prompt` | **inside our production Trino engine** | **yes** (Bedrock/OpenAI) | via generated vectors + similarity | -| **Fenic** (typedef.ai) | PySpark-style **client-side** DataFrame lib with semantic operators (`embed`, `with_cluster_labels` K-means, `analyze_sentiment`, `classify`, `semantic.join`) | a Python process (a Dagster asset) | **yes** (OpenAI/Google/Cohere APIs — cloud only) | K-means clustering built in | -| **StarRocks vector index** | HNSW/IVFPQ index + `approx_cosine_similarity` | inside StarRocks — **not deployed** | **no** (store/search only) | yes (ANN) | - -Key correction to the original framing: **Fenic does not "use" Trino/StarRocks AI functions.** -It has its *own* embedding/semantic operators and runs client-side; it is an alternative to a -hand-rolled Python pipeline, not a way to invoke engine AI functions. StarRocks *stores & -searches* vectors but does **not** generate them. Only Starburst pushes embedding generation -into the engine we already run. +| **Starburst Galaxy AI functions** | in-SQL `starburst.ai.generate_embedding` etc. (Galaxy preview) | inside the SQL engine | yes (Bedrock/OpenAI **wrapper**) | **no — Galaxy-only; StarRocks has no equivalent** | +| **Direct AWS Bedrock client** (boto3) | `bedrock-runtime.invoke_model('amazon.titan-embed-text-v2:0', …)` | an **engine-external** Dagster asset | yes (Bedrock, **in our AWS account**) | **yes — indifferent to the SQL engine** | +| **Fenic** (typedef.ai) | client-side DataFrame lib w/ semantic ops + K-means | an engine-external Dagster asset | yes (OpenAI/Google/Cohere APIs — cloud only) | yes (engine-external) but **cloud-egress**, K-means only | +| **local `sentence-transformers`** | self-hosted model in a Dagster asset | engine-external | yes (local, $0 egress) | yes, but heaviest image | +| **StarRocks vector index** | HNSW/IVFPQ + `approx_cosine_similarity` | inside StarRocks (future) | **no** (store/search only) | it *is* the target — for serving, not generation | + +Two corrections to the earlier framing that matter here: +- **Starburst's `generate_embedding` is only a SQL wrapper over Bedrock models.** We can call the + *same* Bedrock model directly with boto3 from the Dagster layer — keeping the in-account / + PII-safe benefit while dropping the Galaxy dependency. The wrapper buys nothing we can't get + portably. +- **StarRocks does not generate embeddings.** So "do it in the future engine's SQL" is not an + option either. Embedding generation is inherently engine-external; the only question is which + external tool. ## Decision -**Primary: push embedding + model-based sentiment into SQL/dbt via Starburst Galaxy AI -functions, on AWS Bedrock models, over Presidio-redacted text. Keep only clustering + -LLM cluster-labeling as a (now lightweight) Python Dagster asset.** +**Keep all AI/embedding compute in the engine-external orchestration (Dagster/Python) layer, +reading and writing open Iceberg tables — so it is indifferent to whether the SQL engine is +Trino today or StarRocks tomorrow. Generate embeddings by calling AWS Bedrock +(`amazon.titan-embed-text-v2:0`) directly via boto3, not through any engine's SQL AI function.** -Revised pipeline (replaces `feedback_ml_approach.md` §A for the embed/sentiment stages): +Revised pipeline: ``` -int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL] - → feedback_embeddings : generate_embedding() → ARRAY(DOUBLE) in Iceberg [dbt, trino_only] - → sentiment_fk : analyze_sentiment() (+ explicit CSAT seed §E) [dbt, trino_only] - → feedback_clusters : read vectors from Iceberg → UMAP+HDBSCAN [Dagster py asset] - → dim_feedback_category : LLM-label clusters (Bedrock via prompt()) [Dagster py asset] +int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL — portable, no AI funcs] + → feedback_embeddings : Dagster asset, boto3 Bedrock invoke_model [engine-external] + → vectors into Iceberg ARRAY sidecar + → feedback_clusters : Dagster asset, read vectors → UMAP+HDBSCAN [engine-external] + → dim_feedback_category: Dagster asset, LLM-label clusters (Bedrock) [engine-external] + → sentiment_fk : explicit CSAT seed + embedding-kNN (reuse vectors) [portable dbt/py, NO analyze_sentiment] ``` Rationale: -1. **Eliminates the heaviest infra.** No torch/sentence-transformers, no embedding model in a - Docker image. Embedding + sentiment become `trino_only`-tagged dbt models. The Python - surface shrinks to *just* clustering + labeling — which read pre-computed vectors, so the - asset stays tiny (sklearn/UMAP, CPU) and still clones `student_risk_probability`. -2. **Vectors land where dbt already lives.** `generate_embeddings` procedure (or - `generate_embedding()` in an INSERT) writes straight into the `feedback_embeddings` Iceberg - `ARRAY(DOUBLE)` column — no new store, no data movement. -3. **PII posture improves, doesn't regress.** Starburst AI runs against **AWS Bedrock**, which - can be in our own AWS account/region (we already run on AWS: Glue/S3/EKS). Bedrock-in-account - over Presidio-redacted text removes the "third-party egress" objection that drove the - original local-only recommendation. (Presidio stays the deterministic redaction guarantee; - `ai.mask` is an LLM-based masker to *evaluate as an add-on*, not to trust for compliance.) -4. **Already on the engine.** No new service to run or operate — it's a feature of the Trino - cluster we already pay for. - -**Clustering stays Python** because neither Trino nor DuckDB does density clustering, and we -specifically want **HDBSCAN's noise class** to separate one-off complaints from systemic -signal (`feedback_ml_approach.md` §C) — Fenic's `with_cluster_labels` is **K-means only** (no -noise class), so it does not satisfy that requirement. - -## Consequences / caveats (must be resolved before committing) - -1. **Gating question — is the AI-workflows feature enabled on our Galaxy plan?** - `generate_embedding` is **public preview** in Starburst Galaxy. Confirm it's available on - our subscription and that we accept depending on a preview feature for a production pipeline - (API-stability risk). This is the single decision that makes-or-breaks the primary option. -2. **Bedrock model access + config.** Requires configuring an embedding model (e.g. - `bedrock_titan`) and a language model in Galaxy AI Model Access Management, and enabling - Bedrock in our AWS account. New platform setup, but far lighter than a torch service. -3. **DuckDB local-dev / CI incompatibility.** AI functions are Trino-only; the embedding and - `analyze_sentiment` models must carry `tags: ['trino_only']` (existing repo pattern) and - will **not** run under the DuckDB `dev_local` target or the DuckDB-based docs CI. Embedding - is not something we'd want running on every local build anyway (cost), so gating it is - acceptable — but you lose full local reproducibility of those two models. -4. **Cost shifts from one-time-local to per-invocation-provider.** Original: ~$2–16 one-time - local compute. Now: Bedrock embedding cost per record (Titan text embeddings are cheap — - order low-tens-of-dollars for 1.18M short redacted utterances — but it's a provider line - item, and re-embedding must still be avoided via persist-once + `model_version`). Do a - quick Bedrock-pricing sanity check at spec-approval time. -5. **`analyze_sentiment`/`classify` are LLM calls per row.** Fine at MVP volume (198K, batch), - but confirm the batch cost and that per-row LLM sentiment beats the cheaper - explicit-CSAT-seed + embedding-kNN option (`feedback_ml_approach.md` §E) — it may not, in - which case keep sentiment on the kNN path and use Starburst AI only for embedding. +1. **Zero engine lock-in.** Nothing AI-specific touches Trino or StarRocks SQL. The dbt models + stay pure, portable, and continue to compile through the existing Trino/DuckDB/`starrocks__` + dispatch. When transforms move to StarRocks, the embedding path doesn't change at all. +2. **Keeps the PII win without the wrapper.** Bedrock Titan runs in **our own AWS account** + (we're already on AWS: Glue/S3/EKS) over Presidio-redacted text — same posture Rev. 1 gained + from Starburst AI, obtained instead by a direct boto3 call. Auth via the Dagster pod's IAM + role (IRSA) with `bedrock:InvokeModel` — likely **no Vault secret** needed (unlike a + third-party API key). +3. **Light, not heavy.** A boto3 Bedrock client is a thin dependency — **no torch, no + sentence-transformers, no model in the image**. It removes the original spec's heaviest + infra *and* avoids the Galaxy dependency. Batching/retries are a modest amount of code around + `invoke_model` (or use Bedrock batch inference for the 1.18M backfill). +4. **Open landing = free StarRocks path.** Vectors persist in an Iceberg `ARRAY` sidecar + (open format). When StarRocks becomes the serving/OLAP + data-bus engine, it **reads those + Iceberg vectors and builds an HNSW index over them** — a load, not a re-embed. So the + target-state vector-serving tier (StarRocks ANN) is reached without rework, and it is now + **on the intended path**, not a Phase-3 afterthought. +5. **Sentiment stays engine-agnostic.** Use the cheaper explicit-CSAT-seed + embedding-kNN path + (`feedback_ml_approach.md` §E) — which reuses the vectors and needs no per-row LLM call and + no engine AI function. Explicitly **do not** adopt Starburst `analyze_sentiment` (Galaxy-only). +6. **Clustering stays Python** (sklearn/UMAP/**HDBSCAN** for the noise class — one-off vs. + systemic; Fenic's K-means can't do this) — already engine-external and portable. + +## Consequences / caveats + +1. **Slightly more code than a SQL one-liner.** We write a small Bedrock-client Dagster asset + instead of a `generate_embedding` dbt model. That is the price of portability, and it is + small (thin boto3 wrapper cloned from `student_risk_probability`'s asset shape). +2. **Bedrock model access + IAM.** Enable `amazon.titan-embed-text-v2:0` (and a text model for + cluster labels) in our AWS account/region and grant the Dagster role `bedrock:InvokeModel`. + No Galaxy AI feature, no preview dependency. +3. **Cost is per-invocation Bedrock**, but cheap: Titan V2 text embeddings at ~$0.02 / 1M tokens + → low tens of dollars for ~1.18M short redacted utterances, one-time (persist-once + + `model_version`; never re-embed). Use Bedrock **batch inference** for the backfill to cut + cost/throughput pressure. Sanity-check at approval time. +4. **Dimensions choice:** Titan V2 supports 256/512/1024 — pick 256 or 512 for a smaller sidecar + and faster HDBSCAN/HNSW unless retrieval quality needs 1024. Tune on the Zendesk sample. +5. **No StarRocks work now.** StarRocks HNSW is the documented *target* serving tier but is not + built until StarRocks is deployed; MVP uses Iceberg-ARRAY brute-force (fine at ≤1.18M for + batch clustering; the "similar feedback"/RAG serving need is what later justifies the HNSW + index). ## Alternatives (kept, in preference order) -- **Fenic (fallback if the Galaxy AI preview is unavailable/undesired).** Gives - batching/caching/cost-accounting/row-level lineage for the Python path and does embed + - sentiment + K-means in one DataFrame pipeline reading Parquet/S3. Downsides vs. the primary: - still client-side (no engine push-down), **cloud-API egress only** (no self-hosted / no - in-account Bedrock embedding path documented), K-means clustering (no HDBSCAN noise class), - and a new framework dependency. Reasonable middle ground; less aligned than SQL push-down - given we're already on Galaxy. (License: OSS public repo — confirm Apache-2.0 before adopting.) -- **Local `sentence-transformers` in Dagster (original §B).** Now the *last* resort: only if - both the Starburst AI feature and cloud/Bedrock egress are ruled out. Zero per-record - provider cost and no egress, but the heaviest image/infra and the most net-new code. -- **StarRocks vector index — deferred to Phase 3.** Not applicable now (not deployed). When/if - the data bus lands on StarRocks, migrate the `feedback_embeddings` sidecar from Iceberg - `ARRAY(DOUBLE)` brute-force to a StarRocks HNSW index for ANN serving (similar-feedback / RAG - at query time). StarRocks still won't *generate* embeddings — those keep coming from Starburst - AI (or a UDF). Fold into the `afact`/serving work, not the MVP. +- **Fenic** — optional ergonomics wrapper for the Python embedding/label path (batching, caching, + cost-accounting, row-level lineage, K-means + `analyze_sentiment` operators). Still + engine-external and portable, **but** its embedding providers are cloud APIs + (OpenAI/Google/Cohere) with **no in-account Bedrock path documented**, so it reintroduces + third-party egress and is less aligned than a direct Bedrock client on the PII axis; its + clustering is K-means (no noise class). Adopt only if the ergonomics outweigh the egress and + we keep clustering on our own HDBSCAN. (Confirm OSS license before use.) +- **Local `sentence-transformers`** — the zero-egress fallback if in-account Bedrock is somehow + ruled out. Heaviest image/infra; no provider cost. Still engine-external/portable. +- **Starburst Galaxy `generate_embedding` — REJECTED for the critical path.** Galaxy-proprietary, + sits in the SQL layer being migrated off, no StarRocks equivalent. Only reconsider as a throwaway + spike if we needed embeddings before any Dagster/Bedrock wiring existed — not worth the eventual + rewrite. ## Net change to the spec -- `feedback_ml_approach.md` §B default flips: **Starburst Galaxy AI `generate_embedding` - (Bedrock, in-account, on redacted text)** becomes the recommended embedding path; - local self-hosted drops to fallback. -- `feedback_dagster_asset_spec.md`: the `feedback_embeddings` and `feedback_sentiment` stages - move from Python assets to `trino_only` dbt models; the Dagster asset shrinks to - `feedback_clusters` + `feedback_category_proposals` (read vectors, UMAP+HDBSCAN, LLM-label). -- No change to the fact, the dims, the contract, or the business keys — this ADR only changes - *where the vector/sentiment compute runs*, which the schema was deliberately built to be - agnostic about (embeddings live in a sidecar; category/sentiment are late-arriving FKs). +- `feedback_ml_approach.md` §B default: **engine-external Dagster asset calling Bedrock + `amazon.titan-embed-text-v2:0` via boto3**, vectors in an Iceberg `ARRAY` sidecar. (Rev. 1's + "Starburst AI `trino_only` dbt model" is withdrawn for engine lock-in.) +- `feedback_dagster_asset_spec.md`: the embedding stage stays a Dagster asset but is a thin + Bedrock-client call (no torch); sentiment stays the CSAT-seed + kNN path; clustering + labeling + unchanged. No `trino_only` AI dbt models. +- StarRocks HNSW vector index is elevated from a Phase-3 afterthought to the **intended + target vector-serving tier**, reached by loading the open Iceberg vectors (no re-embed). +- Schema, contract, and business keys unchanged — compute location was always meant to be + swappable (vectors in a sidecar; category/sentiment late-arriving FKs), which is exactly what + makes this a compute-only, engine-portable revision. diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md index d67713f5d..26f2d30e7 100644 --- a/docs/design/feedback_dagster_asset_spec.md +++ b/docs/design/feedback_dagster_asset_spec.md @@ -9,16 +9,17 @@ LLM-proposed categories, and sentiment. Grounded in the existing repo orchestrat patterns. **The fact ships without this asset**; this is purely additive (fills `embedding_id`, `category_fk`, `sentiment_fk`, and the `feedback_embeddings` sidecar). -> **REVISED 2026-07-10 — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** -> Because production runs **Starburst Galaxy** (Trino), the **embedding** and **model-based -> sentiment** stages move OUT of this Python asset and INTO `trino_only` dbt models using -> Starburst AI functions (`generate_embedding` → Iceberg `ARRAY(DOUBLE)`; `analyze_sentiment`). -> This asset then shrinks to **only what SQL cannot do: clustering + LLM cluster-labeling** — -> read pre-computed vectors from the Iceberg sidecar → UMAP+HDBSCAN → LLM-label clusters. No -> embedding model in the image (no torch/sentence-transformers), so it stays a small -> sklearn/UMAP CPU asset still cloned from `student_risk_probability`. §2–§6 below describe the -> full-fat Python version; treat the embed/sentiment assets there as the fallback used only if -> the Starburst AI preview is not adopted (ADR "Alternatives"). +> **REVISED 2026-07-10 (rev. 2) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> Because the strategic direction is to **retire Trino for StarRocks**, all AI compute stays +> **engine-external and portable** — no engine-native AI SQL functions (Starburst's are +> Galaxy-only; StarRocks has none). So the embedding stage stays a Dagster asset, but it is a +> **thin boto3 AWS Bedrock call** (`amazon.titan-embed-text-v2:0`), **not** torch/sentence-transformers +> and **not** a Starburst `trino_only` dbt model. Auth via the Dagster pod IAM role +> (`bedrock:InvokeModel`), likely no Vault secret. Vectors land in an open Iceberg `ARRAY` +> sidecar (StarRocks reads it later to build an HNSW index — a load, not a re-embed). Sentiment +> stays the explicit-CSAT + embedding-kNN path (no `analyze_sentiment`). §2–§6 below describe the +> asset shape; substitute the boto3-Bedrock client for the local embedding model where §4 lists +> `sentence-transformers` (that is now the fallback, not the default). --- diff --git a/docs/design/feedback_ml_approach.md b/docs/design/feedback_ml_approach.md index 040e48c7d..5c256d385 100644 --- a/docs/design/feedback_ml_approach.md +++ b/docs/design/feedback_ml_approach.md @@ -38,27 +38,28 @@ so a re-run never duplicates and never mutates the fact grain. Embedding is comp **Decision: one shared embedding, computed once, stored in the `feedback_embeddings` sidecar.** Both clustering and (semantic) sentiment consume it — do not embed twice. -> **REVISED 2026-07-10 — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** -> Production runs Trino on **Starburst Galaxy**, whose in-SQL AI functions -> (`starburst.ai.generate_embedding` → `ARRAY(DOUBLE)`, public preview) can generate -> embeddings **inside the engine we already run**, on **AWS Bedrock in our own AWS account**, -> writing vectors straight into the Iceberg sidecar. That both eliminates the net-new -> torch/sentence-transformers service *and* removes the third-party-egress objection below -> (Bedrock-in-account over Presidio-redacted text). **New default: Starburst AI -> `generate_embedding` as a `trino_only` dbt model.** The local option below drops to a -> last-resort fallback. StarRocks vector functions are irrelevant for now (not deployed) — see -> the ADR for the Phase-3 note. The rest of §B (persist-once, `model_version`, Iceberg ARRAY -> storage, redaction-upstream) is unchanged and applies regardless of who computes the vector. - -- **Model choice (fallback path, if the Starburst AI preview is unavailable/undesired) — - an open/self-hostable model** (e.g. `BAAI/bge-small-en-v1.5` or - `sentence-transformers/all-MiniLM-L6-v2`, 384-dim): even post-redaction, sending ~1.18M - feedback utterances to a *third-party* embedding API is avoidable egress — but note this - concern does **not** apply to Bedrock-in-account via Starburst AI (the new default). A local - sentence-transformers model runs the full corpus in a single batch job on CPU/GPU for - effectively $0 marginal provider cost, at the price of the heaviest image/infra. - - Open question deferred to implementation: GPU vs CPU throughput at 1.18M rows. At - MVP scale (198K) CPU is fine (single-digit minutes to low hours). +> **REVISED 2026-07-10 (rev. 2) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> Strategic direction is to **retire Trino for StarRocks** (transforms + data bus + OLAP +> serving), so **no Galaxy-only functionality may sit on the critical path** — this rules out +> Starburst's `generate_embedding` (Galaxy-proprietary; StarRocks has no embedding-generation +> equivalent). **New default: keep embedding compute engine-external in a Dagster asset that +> calls AWS Bedrock `amazon.titan-embed-text-v2:0` directly via boto3** — in our own AWS account +> (PII-safe over redacted text), a thin dependency (no torch), and indifferent to whether the +> SQL engine is Trino or StarRocks. Vectors land in an open Iceberg `ARRAY` sidecar, which +> StarRocks later reads to build an HNSW index (a load, not a re-embed) — making StarRocks ANN +> the intended serving tier. The rest of §B (persist-once, `model_version`, Iceberg storage, +> redaction-upstream) is unchanged and applies regardless of who computes the vector. + +- **Model choice — default is Bedrock `amazon.titan-embed-text-v2:0`** (dims 256/512/1024 — pick + 256/512 for a smaller sidecar + faster HDBSCAN/HNSW unless retrieval needs 1024), called via + boto3 from the Dagster layer. In-account Bedrock over Presidio-redacted text has no + third-party-egress concern. Use Bedrock **batch inference** for the 1.18M backfill. + - **Fallbacks (ADR):** local `sentence-transformers` (`BAAI/bge-small-en-v1.5` / + `all-MiniLM-L6-v2`, 384-dim) as the $0-egress-but-heaviest option; Fenic as an ergonomics + wrapper (but cloud-egress only). Both stay engine-external/portable. + - Open question deferred to implementation: GPU vs CPU throughput at 1.18M rows (moot for the + Bedrock path — it's an API call; relevant only for the local fallback). MVP scale (198K) is + trivial batch either way. - **Redaction is upstream and mandatory** (design §7): embeddings are computed on the Presidio-redacted text only. Raw text never reaches the embedding step. - **`model_version` is a first-class column** on `feedback_embeddings`. Changing the model From f149ce452719622eb65aa0d461b4b1db212ce33e Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 08:44:47 -0400 Subject: [PATCH 4/7] =?UTF-8?q?docs(feedback):=20ADR=20rev.3=20=E2=80=94?= =?UTF-8?q?=20Fenic=20(Apache-2.0)=20as=20engine-external=20framework?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against fenic source (main): Fenic is Apache-2.0 and its shipped embedding providers are OpenAI/Cohere/Google (Developer+Vertex) — genuinely open source and provider-agnostic, so it's a first-class fit for the portable engine-external layer (not the downgraded footnote rev.2 made it). Correction to the record: there is NO BedrockEmbeddingModel config class yet — "Bedrock" in config.py is only an OpenRouter LLM-routing target (not embeddings), and ROADMAP lists Bedrock as planned. So native in-account Bedrock embeddings via Fenic are roadmap-not-shipped. Decision: use Fenic as the engine-external framework (embed + classify/ sentiment + labeling; batching/caching/lineage free), with the embedding PROVIDER as a PII-policy choice decoupled from the framework — in-account Bedrock (via boto3 today, or contribute the provider; Apache-2.0 allows it) preferred, or a Fenic-native managed provider over redacted text if policy allows. Clustering stays our own sklearn UMAP+HDBSCAN (Fenic is K-means only, no noise class). Vectors → open Iceberg ARRAY; StarRocks HNSW is the intended serving tier (load, not re-embed). No engine-native AI SQL functions; no trino_only AI models. Schema/contract/business-keys unchanged. --- docs/design/README_feedback_aggregation.md | 20 +- docs/design/adr_embedding_compute_strategy.md | 230 +++++++++--------- docs/design/feedback_dagster_asset_spec.md | 24 +- docs/design/feedback_ml_approach.md | 45 ++-- 4 files changed, 164 insertions(+), 155 deletions(-) diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md index 2cc9fd9e8..98edb13cd 100644 --- a/docs/design/README_feedback_aggregation.md +++ b/docs/design/README_feedback_aggregation.md @@ -34,15 +34,17 @@ signals for four audiences (support, engineering, instructors, leadership). tag-seeded categories + CSAT-derived sentiment; embeddings/clustering fill `category_fk`/ `sentiment_fk`/`embedding_id` later. Embeddings persisted **once** (the one adopted lesson from prototype #10793). -4. **Engine-portable AI compute** (revised 2026-07-10 rev. 2, ADR): because the strategic - direction is to **retire Trino for StarRocks**, no Galaxy-only functionality sits on the - critical path. Embedding stays engine-external in a Dagster asset calling **AWS Bedrock - `amazon.titan-embed-text-v2:0` directly via boto3** (in-account → PII-safe over redacted - text; no torch; indifferent to Trino vs. StarRocks). Vectors land in an open Iceberg - `ARRAY` sidecar; **StarRocks HNSW is the intended vector-serving tier**, reached by - loading those vectors (no re-embed). Redaction (Presidio) mandatory pre-embed. Fenic and - local `sentence-transformers` are fallbacks; Starburst `generate_embedding` is **rejected** - for engine lock-in. +4. **Engine-portable AI compute via Fenic** (revised 2026-07-10 rev. 3, ADR): because the + strategic direction is to **retire Trino for StarRocks**, no Galaxy-only functionality sits + on the critical path (Starburst `generate_embedding` **rejected**). AI compute stays + engine-external using **Fenic (Apache-2.0)** in a Dagster asset — batching/caching/lineage for + free, indifferent to Trino vs. StarRocks. **Embedding provider is a PII-policy choice:** + in-account **AWS Bedrock** `amazon.titan-embed-text-v2:0` (best posture — via boto3 today, as + native Fenic Bedrock embeddings are roadmap-not-shipped) or a Fenic-native managed provider + (Cohere/OpenAI/Google) over redacted text. Vectors → open Iceberg `ARRAY` sidecar; + **StarRocks HNSW is the intended vector-serving tier**, reached by loading those vectors (no + re-embed). Clustering stays our own HDBSCAN (Fenic has K-means only). Presidio redaction + mandatory pre-embed. 5. **Superset-first consumption**; support + engineering are the MVP-served audiences (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). diff --git a/docs/design/adr_embedding_compute_strategy.md b/docs/design/adr_embedding_compute_strategy.md index ee3dbbba1..aa26b06d8 100644 --- a/docs/design/adr_embedding_compute_strategy.md +++ b/docs/design/adr_embedding_compute_strategy.md @@ -1,140 +1,134 @@ # ADR: Where feedback embedding / AI inference runs Status: **proposed** · Project: `wp-feedback-aggregation-clustering-system-2e9750` -Date: 2026-07-10 (rev. 2 — engine-portability constraint) · Amends +Date: 2026-07-10 (rev. 3 — Fenic as the engine-external framework) · Amends [`feedback_ml_approach.md`](./feedback_ml_approach.md) §B/§E and [`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md). ## Context -The original ML spec (`feedback_ml_approach.md` §B) proposed a net-new Dagster code location -running `sentence-transformers`/`torch` to embed feedback locally, for the learner-PII posture. -The Dagster explore flagged this as the heaviest part of the system (large image, first ML -service, all-new deps). - -Two facts then reshaped the decision: - -1. **Production runs Trino on Starburst Galaxy** (`src/ol_dbt/profiles.yml`); local dev is - DuckDB; **StarRocks is not deployed** (only `starrocks__` dispatch branches in - `macros/cross_db_functions.sql`). -2. **Strategic direction (owner, 2026-07-10): retire Trino in favour of StarRocks** as the - single engine for warehouse/Iceberg transformations, the "data bus", and OLAP serving. - **Therefore: no Galaxy-only (Starburst-proprietary) functionality may sit on the critical - path** — anything we build on the SQL engine must survive the Trino→StarRocks migration. - -Rev. 1 of this ADR recommended pushing embedding into SQL via Starburst Galaxy's -`generate_embedding`. **Fact 2 vetoes that**: it is a Galaxy-proprietary SQL function living in -exactly the layer being migrated to StarRocks, and StarRocks has **no** embedding-generation -function to migrate it to (StarRocks vector support is *store + ANN search* only — -`cosine_similarity`, HNSW/IVFPQ indexes — not generation). Adopting it would guarantee a -rewrite. This revision removes it from the critical path. - -## The three candidates, disambiguated - -| Candidate | What it is | Runs where | Generates embeddings? | Engine-portable (Trino→StarRocks)? | -|---|---|---|---|---| -| **Starburst Galaxy AI functions** | in-SQL `starburst.ai.generate_embedding` etc. (Galaxy preview) | inside the SQL engine | yes (Bedrock/OpenAI **wrapper**) | **no — Galaxy-only; StarRocks has no equivalent** | -| **Direct AWS Bedrock client** (boto3) | `bedrock-runtime.invoke_model('amazon.titan-embed-text-v2:0', …)` | an **engine-external** Dagster asset | yes (Bedrock, **in our AWS account**) | **yes — indifferent to the SQL engine** | -| **Fenic** (typedef.ai) | client-side DataFrame lib w/ semantic ops + K-means | an engine-external Dagster asset | yes (OpenAI/Google/Cohere APIs — cloud only) | yes (engine-external) but **cloud-egress**, K-means only | -| **local `sentence-transformers`** | self-hosted model in a Dagster asset | engine-external | yes (local, $0 egress) | yes, but heaviest image | -| **StarRocks vector index** | HNSW/IVFPQ + `approx_cosine_similarity` | inside StarRocks (future) | **no** (store/search only) | it *is* the target — for serving, not generation | - -Two corrections to the earlier framing that matter here: -- **Starburst's `generate_embedding` is only a SQL wrapper over Bedrock models.** We can call the - *same* Bedrock model directly with boto3 from the Dagster layer — keeping the in-account / - PII-safe benefit while dropping the Galaxy dependency. The wrapper buys nothing we can't get - portably. -- **StarRocks does not generate embeddings.** So "do it in the future engine's SQL" is not an - option either. Embedding generation is inherently engine-external; the only question is which - external tool. +Original spec (`feedback_ml_approach.md` §B) proposed a net-new Dagster location running +`sentence-transformers`/`torch` to embed locally — the heaviest infra in the system. Two +constraints then reshaped the decision: + +1. **Production runs Trino on Starburst Galaxy**; local dev is DuckDB; **StarRocks is not yet + deployed**. And the **strategic direction is to retire Trino for StarRocks** (warehouse + transforms + data bus + OLAP serving). ⇒ **No Galaxy-only functionality on the critical + path.** This vetoes pushing embedding into SQL via Starburst's `generate_embedding` + (Galaxy-proprietary; StarRocks has no embedding-generation equivalent — its vector support + is *store + ANN search* only). Embedding generation is inherently **engine-external**. +2. **Fenic** ([typedef-ai/fenic](https://github.com/typedef-ai/fenic)) is a viable + engine-external framework for this layer — this revision evaluates it as a first-class option + rather than a footnote. + +### What Fenic actually is (verified against the source, 2026-07-10) + +- **License: Apache-2.0** (`LICENSE` on `main`). Genuinely open source — no Galaxy-style + lock-in. +- **Engine-external**, PySpark-style DataFrame framework with a query engine built for LLM + inference: semantic operators (`embed`, `semantic.classify`, `semantic.analyze_sentiment`, + `semantic.extract`, `semantic.join`, `semantic.reduce`), a native `EmbeddingType`, plus + `with_cluster_labels(by, num_clusters)` (**K-means**), and built-in batching, rate-limiting, + retries, token/cost accounting, and row-level lineage. Reads CSV/Parquet/S3/HF datasets. +- **Embedding providers in the shipped `SessionConfig`:** `OpenAIEmbeddingModel`, + `CohereEmbeddingModel`, `GoogleDeveloperEmbeddingModel`, `GoogleVertexEmbeddingModel` + (language models: OpenAI, Anthropic, Google Developer/Vertex, OpenRouter). So it is + **embedding-provider-agnostic** across those vendors. +- **AWS Bedrock status — important nuance:** there is **no `BedrockEmbeddingModel` config class + today.** In `config.py`, "Bedrock" appears only as an *OpenRouter routing target for language + models* (a third-party hop through openrouter.ai, and for LLMs, not embeddings), and + `ROADMAP.md` lists Bedrock as planned. So **native, in-account Bedrock *embeddings* through + Fenic are not yet available** — but because Fenic is Apache-2.0, a Bedrock embedding provider + can be contributed/added if we require it. (If a Fenic+Bedrock embedding path has landed in a + newer release than what `main` showed here, confirm the exact `SessionConfig` class before + relying on it.) ## Decision -**Keep all AI/embedding compute in the engine-external orchestration (Dagster/Python) layer, -reading and writing open Iceberg tables — so it is indifferent to whether the SQL engine is -Trino today or StarRocks tomorrow. Generate embeddings by calling AWS Bedrock -(`amazon.titan-embed-text-v2:0`) directly via boto3, not through any engine's SQL AI function.** - -Revised pipeline: +**Keep all AI/embedding compute engine-external so it is indifferent to Trino-today vs. +StarRocks-tomorrow, and use Fenic (Apache-2.0) as that engine-external framework** for the +embed → (classify/sentiment) → label pipeline, running in a Dagster asset that reads the dbt +outputs and writes vectors/labels back to open Iceberg. **Clustering stays our own +sklearn/UMAP + HDBSCAN** (Fenic offers only K-means; we need HDBSCAN's noise class to separate +one-off complaints from systemic signal — `feedback_ml_approach.md` §C). ``` -int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL — portable, no AI funcs] - → feedback_embeddings : Dagster asset, boto3 Bedrock invoke_model [engine-external] - → vectors into Iceberg ARRAY sidecar - → feedback_clusters : Dagster asset, read vectors → UMAP+HDBSCAN [engine-external] - → dim_feedback_category: Dagster asset, LLM-label clusters (Bedrock) [engine-external] - → sentiment_fk : explicit CSAT seed + embedding-kNN (reuse vectors) [portable dbt/py, NO analyze_sentiment] +int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL — portable, no AI funcs] + → Fenic pipeline in a Dagster asset: [engine-external, Apache-2.0] + .semantic.embed → EmbeddingType column → Iceberg ARRAY sidecar + .semantic.analyze_sentiment / .classify (optional; see §E note) + → feedback_clusters : our sklearn UMAP+HDBSCAN over the vectors [engine-external] + → dim_feedback_category : LLM-label clusters (Fenic semantic op or a client call) + → sentiment_fk : explicit-CSAT seed + embedding-kNN (default) or Fenic analyze_sentiment ``` -Rationale: -1. **Zero engine lock-in.** Nothing AI-specific touches Trino or StarRocks SQL. The dbt models - stay pure, portable, and continue to compile through the existing Trino/DuckDB/`starrocks__` - dispatch. When transforms move to StarRocks, the embedding path doesn't change at all. -2. **Keeps the PII win without the wrapper.** Bedrock Titan runs in **our own AWS account** - (we're already on AWS: Glue/S3/EKS) over Presidio-redacted text — same posture Rev. 1 gained - from Starburst AI, obtained instead by a direct boto3 call. Auth via the Dagster pod's IAM - role (IRSA) with `bedrock:InvokeModel` — likely **no Vault secret** needed (unlike a - third-party API key). -3. **Light, not heavy.** A boto3 Bedrock client is a thin dependency — **no torch, no - sentence-transformers, no model in the image**. It removes the original spec's heaviest - infra *and* avoids the Galaxy dependency. Batching/retries are a modest amount of code around - `invoke_model` (or use Bedrock batch inference for the 1.18M backfill). -4. **Open landing = free StarRocks path.** Vectors persist in an Iceberg `ARRAY` sidecar - (open format). When StarRocks becomes the serving/OLAP + data-bus engine, it **reads those - Iceberg vectors and builds an HNSW index over them** — a load, not a re-embed. So the - target-state vector-serving tier (StarRocks ANN) is reached without rework, and it is now - **on the intended path**, not a Phase-3 afterthought. -5. **Sentiment stays engine-agnostic.** Use the cheaper explicit-CSAT-seed + embedding-kNN path - (`feedback_ml_approach.md` §E) — which reuses the vectors and needs no per-row LLM call and - no engine AI function. Explicitly **do not** adopt Starburst `analyze_sentiment` (Galaxy-only). -6. **Clustering stays Python** (sklearn/UMAP/**HDBSCAN** for the noise class — one-off vs. - systemic; Fenic's K-means can't do this) — already engine-external and portable. +**Embedding provider is a PII-policy choice, decoupled from the framework:** +- **If in-account AWS Bedrock is a hard requirement** (best PII posture — vectors never leave + our AWS account, over Presidio-redacted text): today that means either (a) **contribute/point a + Bedrock embedding provider into Fenic** (Apache-2.0 allows it; aligns with their roadmap), or + (b) compute the embedding with a thin **boto3 `bedrock-runtime.invoke_model` + (`amazon.titan-embed-text-v2:0`)** step and use Fenic for the *rest* of the semantic pipeline. + Both keep the compute engine-external and portable. +- **If a supported managed provider on redacted text is acceptable** (Cohere / OpenAI / Google + via Fenic's shipped config): simplest path, at the cost of provider egress of + *already-redacted* text. Decide against the learner-PII policy. + +Either way the **vectors land in an open Iceberg `ARRAY` sidecar**, so when StarRocks +becomes the serving/OLAP + data-bus engine it **reads those vectors and builds an HNSW index** +over them (a load, not a re-embed) — **StarRocks ANN is the intended vector-serving tier**, +reached with no rework. dbt models stay pure and portable (existing +`default__`/`duckdb__`/`starrocks__` dispatch); **no engine-native AI SQL functions, no +`trino_only` AI models.** + +## Why Fenic over a hand-rolled boto3 pipeline + +Both are engine-external and portable; Fenic adds, for free, what we'd otherwise hand-build: +automatic batching, rate-limiting, retries, token/cost accounting, response caching, row-level +lineage, and typed semantic operators (`classify`/`analyze_sentiment`/`extract`) we can reuse +for category assignment and sentiment. Apache-2.0 means no lock-in and the option to extend it +(e.g. the Bedrock embedding provider). The one thing it does **not** replace is HDBSCAN — keep +that ours. ## Consequences / caveats -1. **Slightly more code than a SQL one-liner.** We write a small Bedrock-client Dagster asset - instead of a `generate_embedding` dbt model. That is the price of portability, and it is - small (thin boto3 wrapper cloned from `student_risk_probability`'s asset shape). -2. **Bedrock model access + IAM.** Enable `amazon.titan-embed-text-v2:0` (and a text model for - cluster labels) in our AWS account/region and grant the Dagster role `bedrock:InvokeModel`. - No Galaxy AI feature, no preview dependency. -3. **Cost is per-invocation Bedrock**, but cheap: Titan V2 text embeddings at ~$0.02 / 1M tokens - → low tens of dollars for ~1.18M short redacted utterances, one-time (persist-once + - `model_version`; never re-embed). Use Bedrock **batch inference** for the backfill to cut - cost/throughput pressure. Sanity-check at approval time. -4. **Dimensions choice:** Titan V2 supports 256/512/1024 — pick 256 or 512 for a smaller sidecar - and faster HDBSCAN/HNSW unless retrieval quality needs 1024. Tune on the Zendesk sample. -5. **No StarRocks work now.** StarRocks HNSW is the documented *target* serving tier but is not - built until StarRocks is deployed; MVP uses Iceberg-ARRAY brute-force (fine at ≤1.18M for - batch clustering; the "similar feedback"/RAG serving need is what later justifies the HNSW - index). - -## Alternatives (kept, in preference order) - -- **Fenic** — optional ergonomics wrapper for the Python embedding/label path (batching, caching, - cost-accounting, row-level lineage, K-means + `analyze_sentiment` operators). Still - engine-external and portable, **but** its embedding providers are cloud APIs - (OpenAI/Google/Cohere) with **no in-account Bedrock path documented**, so it reintroduces - third-party egress and is less aligned than a direct Bedrock client on the PII axis; its - clustering is K-means (no noise class). Adopt only if the ergonomics outweigh the egress and - we keep clustering on our own HDBSCAN. (Confirm OSS license before use.) -- **Local `sentence-transformers`** — the zero-egress fallback if in-account Bedrock is somehow - ruled out. Heaviest image/infra; no provider cost. Still engine-external/portable. -- **Starburst Galaxy `generate_embedding` — REJECTED for the critical path.** Galaxy-proprietary, - sits in the SQL layer being migrated off, no StarRocks equivalent. Only reconsider as a throwaway - spike if we needed embeddings before any Dagster/Bedrock wiring existed — not worth the eventual - rewrite. +1. **New dependency (Fenic) + provider setup.** Add `fenic` to the new Dagster project's + `pyproject.toml`. Configure the chosen embedding provider (Bedrock-via-boto3/contrib, or a + Fenic-native provider) and credentials (Bedrock → Dagster pod IAM role `bedrock:InvokeModel`, + likely no Vault secret; a managed provider → a Vault-stored API key via the existing + `ConfigurableResource` pattern). +2. **Bedrock-embedding gap (see Context).** If in-account Bedrock embeddings are required *now* + and not yet native to Fenic, use the boto3-embed + Fenic-for-the-rest split, or add the + provider. Don't assume a `BedrockEmbeddingModel` exists without checking the installed version. +3. **Iceberg I/O is not native to Fenic.** Fenic reads CSV/Parquet/S3/HF, not Iceberg directly. + Integration: load the dbt table to a DataFrame (existing `get_dbt_model_as_dataframe` → + Polars) and stage to Parquet/S3 for `session.read.parquet(...)`, or hand Fenic the frame if + in-memory ingestion is supported in the installed version; write vectors out as Parquet and + load to the Iceberg sidecar. Confirm the exact read/write surface at implementation. +4. **Clustering is not Fenic's.** UMAP+HDBSCAN stays a small sklearn step reading the vectors. +5. **Cost/PII depend on the provider, not Fenic.** Titan V2 embeddings ~$0.02/1M tokens → low + tens of $ one-time for ~1.18M redacted utterances; persist-once + `model_version`, never + re-embed; use batch inference for the backfill. A managed provider has similar cost but + egresses redacted text — a policy call. + +## Alternatives (kept) + +- **Direct boto3 Bedrock client, no Fenic** — the minimal engine-external path; use if we don't + want the Fenic dependency. Loses the batching/caching/lineage ergonomics (hand-build them). +- **Local `sentence-transformers`** — zero-egress, no provider cost, but the heaviest image; + fallback only if all managed/Bedrock options are ruled out. Still engine-external/portable; + could even be wired behind Fenic if/when it supports local models. +- **Starburst Galaxy `generate_embedding` — REJECTED for the critical path** (Galaxy-proprietary, + in the layer being migrated off, no StarRocks equivalent). +- **StarRocks vector index** — the *target serving tier*, not a generation option; built once + StarRocks is deployed, by loading the open Iceberg vectors. ## Net change to the spec -- `feedback_ml_approach.md` §B default: **engine-external Dagster asset calling Bedrock - `amazon.titan-embed-text-v2:0` via boto3**, vectors in an Iceberg `ARRAY` sidecar. (Rev. 1's - "Starburst AI `trino_only` dbt model" is withdrawn for engine lock-in.) -- `feedback_dagster_asset_spec.md`: the embedding stage stays a Dagster asset but is a thin - Bedrock-client call (no torch); sentiment stays the CSAT-seed + kNN path; clustering + labeling - unchanged. No `trino_only` AI dbt models. -- StarRocks HNSW vector index is elevated from a Phase-3 afterthought to the **intended - target vector-serving tier**, reached by loading the open Iceberg vectors (no re-embed). +- `feedback_ml_approach.md` §B default: **engine-external Fenic pipeline (Apache-2.0), embedding + provider chosen by PII policy** (in-account Bedrock via boto3/contrib preferred; managed + provider on redacted text acceptable if policy allows). Vectors → Iceberg `ARRAY` sidecar. +- `feedback_dagster_asset_spec.md`: embedding/sentiment run via Fenic in the Dagster asset (not + torch, not a Starburst SQL function); clustering stays sklearn HDBSCAN; no `trino_only` AI models. +- StarRocks HNSW is the **intended** vector-serving tier, reached by loading Iceberg vectors. - Schema, contract, and business keys unchanged — compute location was always meant to be - swappable (vectors in a sidecar; category/sentiment late-arriving FKs), which is exactly what - makes this a compute-only, engine-portable revision. + swappable (vectors in a sidecar; category/sentiment late-arriving FKs). diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md index 26f2d30e7..5a486373d 100644 --- a/docs/design/feedback_dagster_asset_spec.md +++ b/docs/design/feedback_dagster_asset_spec.md @@ -9,17 +9,23 @@ LLM-proposed categories, and sentiment. Grounded in the existing repo orchestrat patterns. **The fact ships without this asset**; this is purely additive (fills `embedding_id`, `category_fk`, `sentiment_fk`, and the `feedback_embeddings` sidecar). -> **REVISED 2026-07-10 (rev. 2) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> **REVISED 2026-07-10 (rev. 3) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** > Because the strategic direction is to **retire Trino for StarRocks**, all AI compute stays > **engine-external and portable** — no engine-native AI SQL functions (Starburst's are -> Galaxy-only; StarRocks has none). So the embedding stage stays a Dagster asset, but it is a -> **thin boto3 AWS Bedrock call** (`amazon.titan-embed-text-v2:0`), **not** torch/sentence-transformers -> and **not** a Starburst `trino_only` dbt model. Auth via the Dagster pod IAM role -> (`bedrock:InvokeModel`), likely no Vault secret. Vectors land in an open Iceberg `ARRAY` -> sidecar (StarRocks reads it later to build an HNSW index — a load, not a re-embed). Sentiment -> stays the explicit-CSAT + embedding-kNN path (no `analyze_sentiment`). §2–§6 below describe the -> asset shape; substitute the boto3-Bedrock client for the local embedding model where §4 lists -> `sentence-transformers` (that is now the fallback, not the default). +> Galaxy-only; StarRocks has none). The embedding/sentiment/labeling stages run via **Fenic +> (Apache-2.0)** inside a Dagster asset — not torch, not a Starburst `trino_only` dbt model. +> **Embedding provider is a PII-policy choice:** in-account AWS Bedrock +> `amazon.titan-embed-text-v2:0` (via boto3 today — native Fenic Bedrock *embeddings* are +> roadmap-not-shipped — or contribute the provider), or a Fenic-native managed provider +> (Cohere/OpenAI/Google) over redacted text if policy allows. Bedrock auth via the Dagster pod +> IAM role (`bedrock:InvokeModel`), likely no Vault secret; a managed provider uses a +> Vault-stored key via the existing `ConfigurableResource` pattern. Vectors land in an open +> Iceberg `ARRAY` sidecar (StarRocks reads it later to build an HNSW index — a load, not a +> re-embed). **Clustering stays our own sklearn UMAP+HDBSCAN** (Fenic has K-means only, no noise +> class). Sentiment defaults to the explicit-CSAT + embedding-kNN path. Fenic does not read +> Iceberg natively — stage via Parquet/S3 or hand it the in-memory frame (confirm at +> implementation). §2–§6 below describe the asset shape; substitute Fenic (or a boto3 Bedrock +> client) for the local `sentence-transformers` model where §4 lists it (now a fallback). --- diff --git a/docs/design/feedback_ml_approach.md b/docs/design/feedback_ml_approach.md index 5c256d385..949d0cc97 100644 --- a/docs/design/feedback_ml_approach.md +++ b/docs/design/feedback_ml_approach.md @@ -38,28 +38,35 @@ so a re-run never duplicates and never mutates the fact grain. Embedding is comp **Decision: one shared embedding, computed once, stored in the `feedback_embeddings` sidecar.** Both clustering and (semantic) sentiment consume it — do not embed twice. -> **REVISED 2026-07-10 (rev. 2) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> **REVISED 2026-07-10 (rev. 3) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** > Strategic direction is to **retire Trino for StarRocks** (transforms + data bus + OLAP > serving), so **no Galaxy-only functionality may sit on the critical path** — this rules out > Starburst's `generate_embedding` (Galaxy-proprietary; StarRocks has no embedding-generation -> equivalent). **New default: keep embedding compute engine-external in a Dagster asset that -> calls AWS Bedrock `amazon.titan-embed-text-v2:0` directly via boto3** — in our own AWS account -> (PII-safe over redacted text), a thin dependency (no torch), and indifferent to whether the -> SQL engine is Trino or StarRocks. Vectors land in an open Iceberg `ARRAY` sidecar, which -> StarRocks later reads to build an HNSW index (a load, not a re-embed) — making StarRocks ANN -> the intended serving tier. The rest of §B (persist-once, `model_version`, Iceberg storage, -> redaction-upstream) is unchanged and applies regardless of who computes the vector. - -- **Model choice — default is Bedrock `amazon.titan-embed-text-v2:0`** (dims 256/512/1024 — pick - 256/512 for a smaller sidecar + faster HDBSCAN/HNSW unless retrieval needs 1024), called via - boto3 from the Dagster layer. In-account Bedrock over Presidio-redacted text has no - third-party-egress concern. Use Bedrock **batch inference** for the 1.18M backfill. - - **Fallbacks (ADR):** local `sentence-transformers` (`BAAI/bge-small-en-v1.5` / - `all-MiniLM-L6-v2`, 384-dim) as the $0-egress-but-heaviest option; Fenic as an ergonomics - wrapper (but cloud-egress only). Both stay engine-external/portable. - - Open question deferred to implementation: GPU vs CPU throughput at 1.18M rows (moot for the - Bedrock path — it's an API call; relevant only for the local fallback). MVP scale (198K) is - trivial batch either way. +> equivalent). **New default: keep embedding compute engine-external, using Fenic (Apache-2.0) +> as the framework** in a Dagster asset reading the dbt outputs and writing vectors to an open +> Iceberg `ARRAY` sidecar — indifferent to whether the SQL engine is Trino or StarRocks, +> and giving batching/caching/cost-accounting/lineage for free. **Embedding provider is a +> PII-policy choice, decoupled from the framework:** in-account **AWS Bedrock** +> `amazon.titan-embed-text-v2:0` (best posture — via boto3 today, since a native Fenic Bedrock +> *embedding* provider is roadmap-not-shipped; or contribute it, Apache-2.0), OR a Fenic-native +> managed provider (Cohere/OpenAI/Google) over Presidio-redacted text if policy allows. Vectors +> in Iceberg → StarRocks later builds an HNSW index over them (a load, not a re-embed) = intended +> serving tier. The rest of §B (persist-once, `model_version`, Iceberg storage, redaction-upstream) +> is unchanged regardless of who computes the vector. + +- **Model choice — recommend Bedrock `amazon.titan-embed-text-v2:0`** (dims 256/512/1024 — pick + 256/512 for a smaller sidecar + faster HDBSCAN/HNSW unless retrieval needs 1024) for the + in-account PII posture over Presidio-redacted text; use Bedrock **batch inference** for the + 1.18M backfill. A Fenic-native managed provider (Cohere/OpenAI/Google) is the simpler + alternative if egress of *redacted* text is acceptable by policy. + - **Fallbacks (ADR):** direct boto3 Bedrock client without Fenic (minimal, hand-build the + batching); local `sentence-transformers` (`BAAI/bge-small-en-v1.5` / `all-MiniLM-L6-v2`, + 384-dim) as the $0-egress-but-heaviest option. All stay engine-external/portable. + - Note: **HDBSCAN clustering stays our own sklearn step** — Fenic offers only K-means + (`with_cluster_labels`), which lacks the noise class we need (§C). Fenic covers embed + + classify/sentiment + labeling; not clustering. + - Open question deferred to implementation: throughput at 1.18M rows (moot for the Bedrock/API + path). MVP scale (198K) is trivial batch either way. - **Redaction is upstream and mandatory** (design §7): embeddings are computed on the Presidio-redacted text only. Raw text never reaches the embedding step. - **`model_version` is a first-class column** on `feedback_embeddings`. Changing the model From 8de39c3d8afc60344e8cdab509697f2447653bac Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 09:39:32 -0400 Subject: [PATCH 5/7] =?UTF-8?q?docs(feedback):=20select=20embedding=20mode?= =?UTF-8?q?l=20by=20effectiveness=20(=C2=A7B.1=20eval=20harness)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner direction: Bedrock/in-account is not a requirement — choose the embedding model on task effectiveness (clustering + retrieval on our own feedback corpus). Egress of Presidio-redacted text to a managed provider is acceptable; redaction stays as data-minimization regardless. Adds §B.1: an effectiveness-driven selection method — use MTEB to narrow, then benchmark the shortlist on a labeled Zendesk sample by clustering agreement (ARI/NMI vs. ticket_tags) + silhouette + human coherence, sweeping Matryoshka dims and cost. Candidate shortlist: Fenic-native managed (gemini-embedding-001, Cohere embed-v4, OpenAI text-embedding-3-large) and self-hosted open (Qwen3-Embedding, BGE-M3) if we host for effectiveness. No hardcoded winner; choice reversible via model_version. Also upgrades LLM semantic-normalization-before-embedding from "rejected default" to a first-class eval arm in §C — 2026 support-ticket-clustering evidence reports it is the single largest lever on cluster quality for short/noisy text; gate adoption by measured lift vs. per-record LLM cost. Syncs ADR + dagster_asset_spec + README notes to rev.4. --- docs/design/README_feedback_aggregation.md | 25 ++-- docs/design/adr_embedding_compute_strategy.md | 21 ++-- docs/design/feedback_dagster_asset_spec.md | 12 +- docs/design/feedback_ml_approach.md | 113 +++++++++++------- 4 files changed, 100 insertions(+), 71 deletions(-) diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md index 98edb13cd..47af3ce89 100644 --- a/docs/design/README_feedback_aggregation.md +++ b/docs/design/README_feedback_aggregation.md @@ -34,17 +34,20 @@ signals for four audiences (support, engineering, instructors, leadership). tag-seeded categories + CSAT-derived sentiment; embeddings/clustering fill `category_fk`/ `sentiment_fk`/`embedding_id` later. Embeddings persisted **once** (the one adopted lesson from prototype #10793). -4. **Engine-portable AI compute via Fenic** (revised 2026-07-10 rev. 3, ADR): because the - strategic direction is to **retire Trino for StarRocks**, no Galaxy-only functionality sits - on the critical path (Starburst `generate_embedding` **rejected**). AI compute stays - engine-external using **Fenic (Apache-2.0)** in a Dagster asset — batching/caching/lineage for - free, indifferent to Trino vs. StarRocks. **Embedding provider is a PII-policy choice:** - in-account **AWS Bedrock** `amazon.titan-embed-text-v2:0` (best posture — via boto3 today, as - native Fenic Bedrock embeddings are roadmap-not-shipped) or a Fenic-native managed provider - (Cohere/OpenAI/Google) over redacted text. Vectors → open Iceberg `ARRAY` sidecar; - **StarRocks HNSW is the intended vector-serving tier**, reached by loading those vectors (no - re-embed). Clustering stays our own HDBSCAN (Fenic has K-means only). Presidio redaction - mandatory pre-embed. +4. **Engine-portable AI compute via Fenic; embedding model chosen by effectiveness** (revised + 2026-07-10 rev. 4, ADR): because the strategic direction is to **retire Trino for StarRocks**, + no Galaxy-only functionality sits on the critical path (Starburst `generate_embedding` + **rejected**). AI compute stays engine-external using **Fenic (Apache-2.0)** in a Dagster asset + — batching/caching/lineage for free, portable across Trino→StarRocks. **Bedrock is not + required; the embedding model is selected by task effectiveness** — MTEB to narrow, then + benchmark the shortlist (Fenic-native `gemini-embedding-001`/Cohere `embed-v4`/OpenAI + `text-embedding-3-large`; or self-hosted Qwen3/BGE-M3) on a labeled Zendesk sample by + clustering agreement/coherence + cost, sweeping Matryoshka dims (`feedback_ml_approach.md` + §B.1). Choice is reversible via `model_version`. Vectors → open Iceberg `ARRAY` sidecar; + **StarRocks HNSW is the intended vector-serving tier**. Clustering stays our own HDBSCAN (Fenic + is K-means only); LLM semantic-normalization-before-embedding is a first-class eval arm (2026 + evidence: biggest lever for short-ticket cluster quality). Presidio redaction mandatory + pre-embed (data-minimization, independent of egress). 5. **Superset-first consumption**; support + engineering are the MVP-served audiences (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). diff --git a/docs/design/adr_embedding_compute_strategy.md b/docs/design/adr_embedding_compute_strategy.md index aa26b06d8..51a8c8afa 100644 --- a/docs/design/adr_embedding_compute_strategy.md +++ b/docs/design/adr_embedding_compute_strategy.md @@ -62,16 +62,17 @@ int__feedback__unioned (redacted text, feedback_pk) [dbt/SQL → sentiment_fk : explicit-CSAT seed + embedding-kNN (default) or Fenic analyze_sentiment ``` -**Embedding provider is a PII-policy choice, decoupled from the framework:** -- **If in-account AWS Bedrock is a hard requirement** (best PII posture — vectors never leave - our AWS account, over Presidio-redacted text): today that means either (a) **contribute/point a - Bedrock embedding provider into Fenic** (Apache-2.0 allows it; aligns with their roadmap), or - (b) compute the embedding with a thin **boto3 `bedrock-runtime.invoke_model` - (`amazon.titan-embed-text-v2:0`)** step and use Fenic for the *rest* of the semantic pipeline. - Both keep the compute engine-external and portable. -- **If a supported managed provider on redacted text is acceptable** (Cohere / OpenAI / Google - via Fenic's shipped config): simplest path, at the cost of provider egress of - *already-redacted* text. Decide against the learner-PII policy. +**Embedding model is chosen by TASK EFFECTIVENESS, not by provider/PII** (owner direction, +2026-07-10: Bedrock/in-account is **not** a requirement; select the model on how well it clusters +and retrieves *our* feedback). Egress of Presidio-redacted text to a managed provider is +acceptable. See `feedback_ml_approach.md` §B.1 for the selection harness (MTEB to narrow → +benchmark the shortlist on a labeled Zendesk sample → pick by clustering agreement/coherence and +cost; sweep Matryoshka dims). Candidate providers: Fenic-native managed +(`gemini-embedding-001` / Cohere `embed-v4` / OpenAI `text-embedding-3-large`) and self-hosted +open (Qwen3-Embedding / BGE-M3 via `sentence-transformers`) if we host for effectiveness. Because +vectors persist with `model_version`, the choice is reversible (re-embed later without touching +the fact). Bedrock (`amazon.titan-embed-text-v2:0` via boto3) remains *a* candidate — no longer +the default, just one option in the bake-off. Either way the **vectors land in an open Iceberg `ARRAY` sidecar**, so when StarRocks becomes the serving/OLAP + data-bus engine it **reads those vectors and builds an HNSW index** diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md index 5a486373d..96c38edc6 100644 --- a/docs/design/feedback_dagster_asset_spec.md +++ b/docs/design/feedback_dagster_asset_spec.md @@ -14,12 +14,12 @@ patterns. **The fact ships without this asset**; this is purely additive (fills > **engine-external and portable** — no engine-native AI SQL functions (Starburst's are > Galaxy-only; StarRocks has none). The embedding/sentiment/labeling stages run via **Fenic > (Apache-2.0)** inside a Dagster asset — not torch, not a Starburst `trino_only` dbt model. -> **Embedding provider is a PII-policy choice:** in-account AWS Bedrock -> `amazon.titan-embed-text-v2:0` (via boto3 today — native Fenic Bedrock *embeddings* are -> roadmap-not-shipped — or contribute the provider), or a Fenic-native managed provider -> (Cohere/OpenAI/Google) over redacted text if policy allows. Bedrock auth via the Dagster pod -> IAM role (`bedrock:InvokeModel`), likely no Vault secret; a managed provider uses a -> Vault-stored key via the existing `ConfigurableResource` pattern. Vectors land in an open +> **Embedding model is chosen by task effectiveness** (Bedrock not required) — MTEB-narrowed +> shortlist benchmarked on a labeled Zendesk sample (`feedback_ml_approach.md` §B.1): Fenic-native +> managed (`gemini-embedding-001`/Cohere `embed-v4`/OpenAI `text-embedding-3-large`) or +> self-hosted open (Qwen3/BGE-M3). Egress of Presidio-redacted text is acceptable. A managed +> provider uses a Vault-stored key via the existing `ConfigurableResource` pattern; a self-hosted +> model needs no secret but a heavier image. Choice is reversible via `model_version`. Vectors land in an open > Iceberg `ARRAY` sidecar (StarRocks reads it later to build an HNSW index — a load, not a > re-embed). **Clustering stays our own sklearn UMAP+HDBSCAN** (Fenic has K-means only, no noise > class). Sentiment defaults to the explicit-CSAT + embedding-kNN path. Fenic does not read diff --git a/docs/design/feedback_ml_approach.md b/docs/design/feedback_ml_approach.md index 949d0cc97..17e57386a 100644 --- a/docs/design/feedback_ml_approach.md +++ b/docs/design/feedback_ml_approach.md @@ -38,51 +38,70 @@ so a re-run never duplicates and never mutates the fact grain. Embedding is comp **Decision: one shared embedding, computed once, stored in the `feedback_embeddings` sidecar.** Both clustering and (semantic) sentiment consume it — do not embed twice. -> **REVISED 2026-07-10 (rev. 3) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** -> Strategic direction is to **retire Trino for StarRocks** (transforms + data bus + OLAP -> serving), so **no Galaxy-only functionality may sit on the critical path** — this rules out -> Starburst's `generate_embedding` (Galaxy-proprietary; StarRocks has no embedding-generation -> equivalent). **New default: keep embedding compute engine-external, using Fenic (Apache-2.0) -> as the framework** in a Dagster asset reading the dbt outputs and writing vectors to an open -> Iceberg `ARRAY` sidecar — indifferent to whether the SQL engine is Trino or StarRocks, -> and giving batching/caching/cost-accounting/lineage for free. **Embedding provider is a -> PII-policy choice, decoupled from the framework:** in-account **AWS Bedrock** -> `amazon.titan-embed-text-v2:0` (best posture — via boto3 today, since a native Fenic Bedrock -> *embedding* provider is roadmap-not-shipped; or contribute it, Apache-2.0), OR a Fenic-native -> managed provider (Cohere/OpenAI/Google) over Presidio-redacted text if policy allows. Vectors -> in Iceberg → StarRocks later builds an HNSW index over them (a load, not a re-embed) = intended -> serving tier. The rest of §B (persist-once, `model_version`, Iceberg storage, redaction-upstream) -> is unchanged regardless of who computes the vector. - -- **Model choice — recommend Bedrock `amazon.titan-embed-text-v2:0`** (dims 256/512/1024 — pick - 256/512 for a smaller sidecar + faster HDBSCAN/HNSW unless retrieval needs 1024) for the - in-account PII posture over Presidio-redacted text; use Bedrock **batch inference** for the - 1.18M backfill. A Fenic-native managed provider (Cohere/OpenAI/Google) is the simpler - alternative if egress of *redacted* text is acceptable by policy. - - **Fallbacks (ADR):** direct boto3 Bedrock client without Fenic (minimal, hand-build the - batching); local `sentence-transformers` (`BAAI/bge-small-en-v1.5` / `all-MiniLM-L6-v2`, - 384-dim) as the $0-egress-but-heaviest option. All stay engine-external/portable. - - Note: **HDBSCAN clustering stays our own sklearn step** — Fenic offers only K-means - (`with_cluster_labels`), which lacks the noise class we need (§C). Fenic covers embed + - classify/sentiment + labeling; not clustering. - - Open question deferred to implementation: throughput at 1.18M rows (moot for the Bedrock/API - path). MVP scale (198K) is trivial batch either way. +> **REVISED 2026-07-10 (rev. 4) — see [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md).** +> Compute stays engine-external via **Fenic (Apache-2.0)** in a Dagster asset, writing vectors to +> an open Iceberg `ARRAY` sidecar (portable across Trino→StarRocks; StarRocks later indexes +> those vectors with HNSW). **Bedrock/in-account is NOT a requirement** — the **embedding model is +> chosen by task effectiveness** (clustering + retrieval on OUR feedback corpus), with egress of +> Presidio-redacted text to a managed provider acceptable. Model selection is specified as an +> evaluation below, not a fixed pick. Persist-once, `model_version`, Iceberg storage, and +> mandatory upstream redaction are unchanged. + +### B.1 Embedding-model selection — by effectiveness, on our own labeled corpus + +Selection principle (industry consensus): **use the MTEB leaderboard to *narrow*, then benchmark +the shortlist on our own labeled Zendesk sample — the decisive metric is performance on our +corpus, not the public average.** Because embeddings are persisted with `model_version` (below), +the choice is **reversible**: re-embed with a better model later without touching the fact. + +**Candidate shortlist (narrow with current MTEB *clustering + retrieval* standings at eval time; +these move monthly).** Two tiers: + +| Tier | Candidates (2026) | Via | Notes | +|---|---|---|---| +| Managed (Fenic-native) | Google `gemini-embedding-001`; Cohere `embed-v4`; OpenAI `text-embedding-3-large` | Fenic `GoogleVertex/GoogleDeveloper/Cohere/OpenAI EmbeddingModel` | gemini + Cohere v4 currently edge OpenAI on MTEB; all support **Matryoshka dim truncation** (test 256/512/1024 — smaller = faster HDBSCAN/HNSW + smaller sidecar, usually minimal quality loss). Egress of redacted text. | +| Self-hosted open (top of MTEB) | Qwen3-Embedding; BGE-M3; NV-Embed-v2 | `sentence-transformers` (outside Fenic's provider set) | Currently top the MTEB average / best open quality-cost (BGE-M3); $0 marginal cost, no egress, full control — at the price of hosting a model (heavier Dagster image / a GPU batch). Include if we're willing to self-host for effectiveness. | + +**Evaluation harness (run once on a labeled sample, ~2–5k tickets):** +1. **Label set:** reuse the existing structure as ground truth — Zendesk `ticket_tags` / + `group_name` give a free (noisy) cluster/label reference; optionally hand-label a few hundred + for a cleaner set. +2. **Task-aligned metrics, not MTEB average:** + - *Clustering* (our primary task): run the §C pipeline (UMAP+HDBSCAN) on each candidate's + vectors; score **silhouette** + **agreement with the tag reference** (adjusted Rand / + normalized mutual information) + a small **human-coherence** rating of the top clusters. + - *Retrieval* (future "similar feedback"/RAG serving): nearest-neighbour precision@k on + tag-matched pairs. +3. **Also sweep dimension** (Matryoshka 256/512/1024) and **cost/latency per 1M** as tie-breakers. +4. **Pick** the model×dim that maximizes clustering agreement/coherence at acceptable cost. + +**Starting default for the eval** (so implementation isn't blocked on the bake-off): a strong +current all-rounder available via Fenic — `gemini-embedding-001` or Cohere `embed-v4` at 512-dim +— with `text-embedding-3-large` as the "safe baseline" comparison and BGE-M3 as the self-hosted +comparison. Let the harness decide; do not hardcode a winner in the spec. + +- **HDBSCAN clustering stays our own sklearn step** — Fenic offers only K-means + (`with_cluster_labels`), which lacks the noise class we need (§C). Fenic covers embed + + classify/sentiment + labeling; not clustering. - **Redaction is upstream and mandatory** (design §7): embeddings are computed on the - Presidio-redacted text only. Raw text never reaches the embedding step. -- **`model_version` is a first-class column** on `feedback_embeddings`. Changing the model - = new `model_version` rows, old ones retained until re-cluster completes (no in-place - overwrite → no torn state). -- **Vector storage:** at ~1.18M × 384 float32 ≈ 1.8 GB. Store as an Iceberg `ARRAY` - column in `feedback_embeddings` for the MVP (no new operational service; aligns with the - lake). Revisit a dedicated vector index (StarRocks vector column, S3 Vectors, Qdrant) - only when an online nearest-neighbour/serving need appears — the batch clustering job - reads the whole set into memory, which is fine at this scale. This defers the RFC's - "embedding model & vector store" open question to a reversible, low-cost default. - -**Rejected:** re-embedding on every run (the prototype #10793 flaw the RFC explicitly -fixes); semantic-summary-before-embedding as a default (see §C, treated as a hypothesis -to test, not a baseline — an LLM call per record turns a ~$2–16 job into a per-record LLM -cost across 1M+ records and can distort short-text sources). + Presidio-redacted text only. Raw text never reaches the embedding step. (Redaction remains + required even though provider egress is now acceptable — it is a data-minimization guarantee, + not just an egress control.) +- **`model_version` is a first-class column** on `feedback_embeddings`. Changing the model (or the + dimension) = new `model_version` rows, old retained until re-cluster completes (no torn state). + This is what makes the model choice reversible and the eval low-risk. +- **Vector storage:** ~1.18M × (256–1024) float32 ≈ 1.2–4.8 GB. Store as an Iceberg `ARRAY` + column for the MVP (no new service; the batch clustering job reads the set into memory — fine at + this scale). StarRocks HNSW becomes the serving-tier index once deployed (ADR). + +**Rejected:** re-embedding on every run (the prototype #10793 flaw the RFC fixes). +**Upgraded from "rejected" to "evaluate seriously" (new evidence, 2026-07):** LLM +**semantic-normalization before embedding**. Recent support-ticket-clustering literature reports it +is *the single largest contributor to cluster quality* on short/noisy ticket text (improved +silhouette + human-rated coherence over baselines) — stronger than the RFC's earlier skeptical +stance. Treat it as a **first-class arm of the §C evaluation**, not a default: it adds an LLM call +per record (cost), so gate by measured lift vs. cost, and expect it to help most on the long noisy +Zendesk descriptions (avg ~1,000 chars) and least on already-short sources (tutor ~58, ORA ~140). --- @@ -107,6 +126,12 @@ cohesion signal that lets a human say "this is recurring." raw high-dim space). `n_neighbors`/`min_cluster_size` are the two knobs to tune on a Zendesk sample; `min_cluster_size` is effectively the "how many tickets before we call it systemic" threshold and should be a config, not hardcoded. +- **Pre-embedding LLM semantic-normalization is a first-class eval arm here** (see §B.1): recent + support-ticket-clustering evidence (2026) reports it is the single largest lever on cluster + quality for short/noisy ticket text. Evaluate `normalize→embed→cluster` against `embed→cluster` + on the labeled sample (silhouette + tag-agreement + human coherence); adopt only where the + measured lift justifies the per-record LLM cost — likely worth it for the long Zendesk + descriptions, likely not for already-short sources. - **Re-clustering is cheap and expected:** each run writes a new `cluster_run_id`; the sidecar keeps prior runs. `dim_feedback_category` (curated) only advances when a human approves labels from a run (design §4a), decoupling churny clustering from the stable From 651d1284328a07ba536ee3226ed4f54b2c98ab81 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 11:56:03 -0400 Subject: [PATCH 6/7] docs(feedback): address PR review feedback (sentry + Copilot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - feedback_zendesk_mvp_spec §8: compound uniqueness test referenced source_slug, which is not a fact column; use ['feedback_source_fk', 'source_record_id'] (both exist; same business grain since feedback_source_fk = surrogate_key([source_slug])). Drop the stray "source_slug equivalent" from the not_null list. (sentry) - feedback_dimensional_model §2/§6: align business-key terminology to the rest of the spec set — feedback_source/source_natural_key → source_slug/source_record_ref. (Copilot) - feedback_dimensional_model §8: remove stray leading "+" diff artifact that rendered oddly in the build-path prose. (Copilot) - feedback_dagster_asset_spec §5: type the PrivateAttr in the LLMClientFactory example (_client: Anthropic | None) per Pydantic v2 + repo convention, so the client-caching pattern is correct. (sentry) --- docs/design/feedback_dagster_asset_spec.md | 2 +- docs/design/feedback_dimensional_model.md | 16 +++++++++------- docs/design/feedback_zendesk_mvp_spec.md | 10 ++++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/docs/design/feedback_dagster_asset_spec.md b/docs/design/feedback_dagster_asset_spec.md index 96c38edc6..b72474c31 100644 --- a/docs/design/feedback_dagster_asset_spec.md +++ b/docs/design/feedback_dagster_asset_spec.md @@ -147,7 +147,7 @@ class LLMClientFactory(ConfigurableResource): vault_mount_point: str = Field(default="secret-data") vault_secret_path: str = Field(default="pipelines/feedback-llm") # NEW Vault path to provision vault_secret_key: str = Field(default="api_key") - _client = PrivateAttr(default=None) + _client: Anthropic | None = PrivateAttr(default=None) def get_client(self): if self._client is None: diff --git a/docs/design/feedback_dimensional_model.md b/docs/design/feedback_dimensional_model.md index 9aaa822ea..acc2398be 100644 --- a/docs/design/feedback_dimensional_model.md +++ b/docs/design/feedback_dimensional_model.md @@ -35,7 +35,7 @@ excluded (filter to `*.created`). ``` -- surrogate + conformed FKs -feedback_pk -- generate_surrogate_key([feedback_source, source_natural_key]) (see §6) +feedback_pk -- generate_surrogate_key([source_slug, source_record_ref]) (see §6) feedback_source_fk -> dim_feedback_source.feedback_source_pk user_fk -> dim_user.user_pk (nullable; anonymous/aggregate sources) platform_fk -> dim_platform.platform_pk (nullable; non-course sources e.g. Zendesk) @@ -161,11 +161,12 @@ tracking logs share ingress semantics. ## 6. Business-key strategy (migration-proof) -`feedback_pk = generate_surrogate_key([feedback_source, source_natural_key])` where `source_natural_key` is a -**stable business identifier from the source system**, never a warehouse/Airbyte/Postgres row PK: +`feedback_pk = generate_surrogate_key([source_slug, source_record_ref])` where `source_record_ref` is a +**stable business identifier from the source system**, never a warehouse/Airbyte/Postgres row PK +(terminology matches the event contract + Zendesk MVP spec): -| Source | source_natural_key | -|--------|--------------------| +| Source | source_record_ref | +|--------|-------------------| | Zendesk | `ticket_id` | | edX forum | `post_id` | | Learn AI tutor | `thread_id` + `checkpoint_pk` (or message index) | @@ -197,8 +198,9 @@ raw____… (existing per source; new: raw__learn → tfact_feedback (resolve conformed FKs, generate feedback_pk §6) → afact_feedback_cluster_daily (aggregate fact: cluster × category × sentiment × date × source — strategic rollup) ``` -Clustering/labeling jobs (Dagster asset) read `int__feedback__unioned` (redacted) → write `feedback_embeddings` -+ `dim_feedback_category` + `category_fk`/`sentiment_fk` back onto the fact (late-arriving update). +Clustering/labeling jobs (Dagster asset) read `int__feedback__unioned` (redacted) and write +`feedback_embeddings`, `dim_feedback_category`, and `category_fk`/`sentiment_fk` back onto the fact +(late-arriving update). --- diff --git a/docs/design/feedback_zendesk_mvp_spec.md b/docs/design/feedback_zendesk_mvp_spec.md index 85d2d0359..306da2e47 100644 --- a/docs/design/feedback_zendesk_mvp_spec.md +++ b/docs/design/feedback_zendesk_mvp_spec.md @@ -209,14 +209,16 @@ sidecar. This ordering lets the fact ship first. ## 8. Tests / contract (`_dim__models.yml` entries) Mirror the `tfact_discussion_events` yml style: -- Per-column `not_null` on: `feedback_pk`, `feedback_source_fk`, `source_slug` equivalent, - `source_record_id`, `feedback_occurred_at`, `time_fk`, `date_fk`. +- Per-column `not_null` on: `feedback_pk`, `feedback_source_fk`, `source_record_id`, + `feedback_occurred_at`, `time_fk`, `date_fk`. - `unique` on `feedback_pk`. - Nullable (description-only, no not_null): `user_fk`, `courserun_fk`, `platform_fk`, `organization_fk`, `category_fk`, `sentiment_fk`, `embedding_id`. - Model-level `dbt_expectations.expect_compound_columns_to_be_unique` on - `['source_slug', 'source_record_id']` (belt-and-suspenders alongside the `feedback_pk` - unique test — matches the precedent's compound-uniqueness convention). + `['feedback_source_fk', 'source_record_id']` (belt-and-suspenders alongside the `feedback_pk` + unique test — matches the precedent's compound-uniqueness convention; both columns exist on the + fact, and `feedback_source_fk = generate_surrogate_key([source_slug])` so this is the same + business grain as `[source_slug, source_record_ref]`). - `relationships` tests from each `*_fk` to its dim PK (richer-contract style, as `dim_course_run` does). - New dims get their own entries; `dim_feedback_category` gets a `unique` on `category_slug`. From 9b0a34f89cb004be5e5c8c01aead5ea999a49f98 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 10 Jul 2026 14:10:07 -0400 Subject: [PATCH 7/7] =?UTF-8?q?docs(feedback):=20surface=20options=20?= =?UTF-8?q?=E2=80=94=20Superset=20/=20Marimo=20webapp=20/=20net-new=20app?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The consumption surface was written Superset-first; broaden it to a real per-audience choice, since the model is surface-agnostic (every surface just reads the modeled tables). Add Marimo notebook-as-webapp as the interactive/ curation/prototyping surface and net-new app as the write-back/embedding option, each grounded in tooling the platform already operates: - Superset is managed as code in src/ol_superset/ with programmatic RLS (apply_rls.py) — lowest-effort MVP dashboards. - images/marimo-jupyterlab/ already runs marimo with Keycloak OIDC + per-user Trino token + IRSA, so a deployed notebook-as-webapp reuses that plumbing and can push row-level authz to the engine (Lakekeeper/Cedar) instead of the app. Recommend phased/audience-matched: Superset for MVP trend/cluster dashboards, a Marimo webapp for cluster exploration + the category-curation loop and to prototype the UX, net-new app only when write-back/product-embedding justifies it. Choice stays reversible. Updates §2/§3/§4/§5 + README. --- docs/design/README_feedback_aggregation.md | 12 +++- docs/design/feedback_consumption_ux_spec.md | 70 ++++++++++++++------- 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/docs/design/README_feedback_aggregation.md b/docs/design/README_feedback_aggregation.md index 47af3ce89..577ce1c29 100644 --- a/docs/design/README_feedback_aggregation.md +++ b/docs/design/README_feedback_aggregation.md @@ -18,7 +18,7 @@ signals for four audiences (support, engineering, instructors, leadership). | [`feedback_zendesk_mvp_spec.md`](./feedback_zendesk_mvp_spec.md) | Build-ready Zendesk MVP: exact dbt models/columns `int__feedback__zendesk → __unioned → tfact_feedback` + 3 dims + tests | (MVP implementation) | | [`feedback_ml_approach.md`](./feedback_ml_approach.md) | Embedding (local, PII-safe), clustering (UMAP+HDBSCAN), category discovery (seed + LLM-label), sentiment (explicit + kNN/classifier) | `...clustering-...a1d7d6`, `...category-...550aba`, `...sentiment-...92988e` | | [`feedback_dagster_asset_spec.md`](./feedback_dagster_asset_spec.md) | Batch ML Dagster asset cloned from `student_risk_probability`; Vault-backed LLM resource; net-new deps; scheduling | (ML pipeline orchestration) | -| [`feedback_consumption_ux_spec.md`](./feedback_consumption_ux_spec.md) | Audiences × altitude, Superset-first surfaces, per-persona actions, access control | `...ui-ux-...476d23` | +| [`feedback_consumption_ux_spec.md`](./feedback_consumption_ux_spec.md) | Audiences × altitude, surface options (Superset / Marimo notebook-as-webapp / net-new app), per-persona actions, access control | `...ui-ux-...476d23` | | [`adr_embedding_compute_strategy.md`](./adr_embedding_compute_strategy.md) | ADR: where embedding/AI inference runs — Starburst Galaxy AI functions (Bedrock, in-SQL) vs. Fenic vs. local vs. StarRocks | (revises embedding default) | ## Key decisions (spec phase) @@ -48,8 +48,14 @@ signals for four audiences (support, engineering, instructors, leadership). is K-means only); LLM semantic-normalization-before-embedding is a first-class eval arm (2026 evidence: biggest lever for short-ticket cluster quality). Presidio redaction mandatory pre-embed (data-minimization, independent of egress). -5. **Superset-first consumption**; support + engineering are the MVP-served audiences - (Zendesk is not course-scoped, so instructors wait for Phase 2 sources). +5. **Consumption surface is an open, per-audience choice** — Superset dashboard, deployed + **Marimo notebook-as-webapp**, or a net-new app; all read the same modeled tables, so the + choice is reversible. Recommended phasing: Superset for MVP trend/cluster dashboards (RLS + already solved in `src/ol_superset/`), a Marimo notebook-as-webapp (existing image + + Keycloak/Trino/IRSA) for interactive exploration + the category-curation loop and to + prototype the UX, and a net-new app only when write-back/product-embedding justifies it. + Support + engineering are the MVP-served audiences (Zendesk is not course-scoped, so + instructors wait for Phase 2 sources). ## Phasing diff --git a/docs/design/feedback_consumption_ux_spec.md b/docs/design/feedback_consumption_ux_spec.md index 3de34f586..3344f93f6 100644 --- a/docs/design/feedback_consumption_ux_spec.md +++ b/docs/design/feedback_consumption_ux_spec.md @@ -31,15 +31,37 @@ support/eng), **course-scoped** (filtered fact, instructors), **strategic rollup ## 2. Surface — where the experience lives -**Recommendation: start in Superset (BI), not a custom app.** Rationale: -- The warehouse already exposes facts to Superset; zero new app infrastructure (matches the - RFC's "effectively zero new infrastructure" MVP posture). -- Support/eng/leadership are already BI consumers; instructors are the only audience that - might warrant an embedded surface later. -- Row-level security in Superset covers the instructor courserun-scoping and the - leadership aggregates-only rule. - -**MVP surfaces (Superset dashboards):** +**The surface is an open choice, and it need not be one surface for all audiences.** Because the +fact + dims are the substrate and every surface just *reads the modeled tables*, the surface is a +swappable, reversible consumer — pick per audience by interactivity and write-back need, and you +can run more than one against the same tables. Three viable options, all grounded in tooling the +platform **already operates**: + +| Option | What it is | Already in-repo? | Best for | Auth / row-level access | Interactivity & write-back | Effort | +|---|---|---|---|---|---|---| +| **Superset dashboard** | Managed BI; dashboards/charts/datasets as code | **Yes** — `src/ol_superset/` (`ol-superset` CLI export/validate/sync/promote) + `dg_projects/lakehouse/.../assets/superset.py`; **programmatic RLS** via `apply_rls.py` | Leadership trend dashboards; support/eng cluster tables with drill-through; anything chart-shaped and read-mostly | **RLS already solved** (per-role row filters) — covers instructor courserun-scoping + leadership aggregates-only | Rich filtering/drill; **little/no write-back**; bespoke interactions are hard | **Lowest** — a new dataset + dashboards on the fact | +| **Marimo notebook-as-webapp** | A reactive Python notebook served as an app (`marimo run`) | **Partly** — `images/marimo-jupyterlab/` image exists (marimo + Keycloak OIDC + per-user Trino token + IRSA to S3/Glue), currently in JupyterHub *notebook* mode; serving one as an app is a small new deploy reusing that image/auth | Analyst/eng **interactive cluster exploration**, semantic-search over clusters, the **category-curation loop**, and **prototyping the UX** before committing to an app | Runs queries **as the user** (Keycloak OIDC → Trino/StarRocks token) → can push **row-level authz to the engine** (Lakekeeper/Cedar) rather than reimplement it | High interactivity; **Python** so it can call the embedding/cluster/LLM code directly; light write-back via widgets (e.g. approve labels) | **Medium** — write a notebook; add an app-serve deploy behind APISIX | +| **Net-new application** | A purpose-built web app | No | **Instructor-embedded** views (in MIT Learn), **support triage/route** workflows at scale, product integration | Full control via the org's **APISIX/Keycloak**; bespoke per-persona policies | Full custom UX + rich **write-back** workflows | **Highest** — a service to build, deploy, own | + +**Recommendation — phased and audience-matched, not one surface:** +1. **MVP (fastest value): Superset** for leadership trends and the support/eng cluster tables — + it's already operated, RLS is already solved as code, and it matches the RFC's + "near-zero new infrastructure" posture. Datasets point at `tfact_feedback` / + `afact_feedback_cluster_daily` / the dims. +2. **Interactive + curation surface: a deployed Marimo notebook-as-webapp** — the right home for + cluster exploration, semantic search, and the **category-curation loop** (approve/merge + LLM-proposed labels), and the **cheapest way to prototype the real UX** before deciding whether + a net-new app is warranted. It reuses the existing marimo image + Keycloak/Trino/IRSA plumbing, + and being Python it can call the same embedding/clustering/label code the pipeline uses. +3. **Net-new app: only when write-back or product-embedding justifies it** — e.g. instructor-facing + views inside MIT Learn, or support triage-and-routing at scale. Defer until the Marimo prototype + has shown what the workflow actually needs; the app then formalizes a proven UX rather than + guessing. + +This ordering keeps commitment low (all three read the same tables) and treats Marimo as the +discovery vehicle between "a dashboard is enough" and "we need a real app." + +**MVP surface content (Superset unless noted):** 1. **Support triage** — cluster list ranked by size/recency, filterable by `source_status`/`source_priority`/`source_channel`/`source_tags`, each cluster drill-through to constituent (redacted) tickets with `source_url` deep-links back to @@ -52,10 +74,8 @@ support/eng), **course-scoped** (filtered fact, instructors), **strategic rollup 4. **Instructor course view** — feedback filtered to a courserun; **Phase 2** (needs course-scoped sources: forum/tutor/ORA — Zendesk MVP is not course-scoped, so instructors get nothing from the MVP slice. Documented gap.) - -**When to graduate to an embedded/custom app:** only if write-back actions (§3) outgrow -Superset — e.g. support wants in-context triage/routing, or eng wants one-click issue -filing. Treat that as a deliberate later decision, not MVP. +5. **Cluster exploration + curation** (Marimo) — interactive nearest-neighbour/semantic browse of + clusters and the label-approval loop; the one surface where MVP write-back genuinely lives. --- @@ -73,17 +93,21 @@ preclude it. Documented target actions (open question, RFC): | Curator (any) | — | approve/merge/deprecate `dim_feedback_category` labels (the curation loop) | The **category curation loop** (approve LLM-proposed labels) is the one write-back that IS -needed early — it can live as a lightweight admin surface or even a manual dbt-seed edit at -MVP, upgrading to a UI when volume warrants (`feedback_ml_approach.md` §D). +needed early. Its natural MVP home is the **Marimo notebook-as-webapp** (§2) — Python widgets over +`dim_feedback_category`, reusing the same code path as the labeling pipeline — with a manual +dbt-seed edit as the zero-UI fallback. It upgrades into a net-new app only if/when the other +write-back workflows (§3) do. --- ## 4. Access control (ties to design §7 + Lakekeeper/Cedar) +Row-level access can be enforced at **two layers**, and the surface choice (§2) decides which: +- **Surface-layer (Superset):** per-role **RLS is already solved as code** (`src/ol_superset/.../apply_rls.py`) — instructor rows filtered to their `courserun_fk`, leadership pointed at the aggregate fact only. +- **Engine-layer (Marimo / net-new app):** these run queries **as the user** via the Keycloak OIDC → Trino/StarRocks token (the marimo image already does this), so authz can be **pushed to the engine (Lakekeeper/Cedar)** instead of reimplemented in the app — one policy set governs every surface. This aligns with the platform's authz direction and is the preferred model for non-Superset surfaces. + - **Redacted text only** is exposed in any consumption surface (raw text stays in `raw`/`stg` under existing PII classification + Lakekeeper/Cedar authz — design §7). -- **Instructor** dashboards are row-filtered to their `courserun_fk` (Superset RLS or a - Cedar policy on the fact). - **Leadership** sees the aggregate fact only (no row-level text) — enforced by pointing their dashboards at `afact_feedback_cluster_daily`, not `tfact_feedback`. - Re-applying PII classification + Lakekeeper/Cedar authz at the new fact/dims is tracked by @@ -94,8 +118,10 @@ MVP, upgrading to a UI when volume warrants (`feedback_ml_approach.md` §D). ## 5. What the MVP consumption slice actually delivers Given the MVP is Zendesk-only and not course-scoped: **support + engineering get working -cluster-triage dashboards; leadership gets Zendesk-only category/sentiment trends; -instructors get nothing until Phase 2 course-scoped sources land.** That is the honest -scope — the model supports all four, but only two-and-a-half audiences are served by the -first data slice. This is acceptable because support/eng recurring-issue triage is the -primary stated motivation. +cluster-triage dashboards (Superset); a curator gets the label-approval loop (Marimo); +leadership gets Zendesk-only category/sentiment trends; instructors get nothing until Phase 2 +course-scoped sources land.** That is the honest scope — the model supports all four audiences and +all three surfaces, but only two-and-a-half audiences are served by the first data slice. This is +acceptable because support/eng recurring-issue triage is the primary stated motivation. The +surface decision stays reversible: start with Superset + a Marimo curation/exploration notebook, +and only build a net-new app once the Marimo prototype proves the workflow warrants it.