TIG-254: Move selected tagger into backend code (corrected) - #34
TIG-254: Move selected tagger into backend code (corrected)#34yubimamiya wants to merge 2 commits into
Conversation
DIodide
left a comment
There was a problem hiding this comment.
Thanks for this — the core approach is solid and the data work is genuinely clean: all 22 tag embeddings are present, correctly 1536-dim, and match the new enum exactly. seed.ts is fully consistent with the renamed enums too.
Holding off on approval though, because the enum rename isn't propagated to the frontend and that breaks the build. Details below.
Important
This branch's history was rewritten. A credential was committed in the first commit; I rebuilt both commits without it and force-pushed. The resulting tree is byte-identical to what you had (git diff between old and new head is empty) and your authorship is preserved — but please run git fetch origin && git reset --hard origin/yubi_dev before your next push, otherwise you'll restore the old history. The credential itself should be treated as burned and rotated regardless, since this repo is public.
Blocking
1. Breaks the TypeScript build
Typechecked both sides in clean worktrees:
tsc --noEmit in apps/web |
|
|---|---|
main (c6f8a4c) |
0 errors |
| this branch | 2 errors |
src/actions/users.ts(35,7): error TS2769: No overload matches this call.
src/actions/users.ts(203,9): error TS2769: No overload matches this call.
Type '"academic"' is not assignable to ... Did you mean '"academics"'?
2. The enum rename isn't propagated to apps/web
eventTagEnum and orgCategoryEnum were rewritten (free-food→free food, academic→academics, cultural→culture, art split into visual arts/performing arts, workshop/speaker dropped, plus 6 new values). 11 files still use the old literals and none are touched by this PR:
src/actions/users.tssrc/app/(app)/events/create/create-event-form.tsxsrc/app/(app)/explore/explore-client.tsxsrc/app/(app)/map/_lib/map-helpers.tssrc/app/(app)/orgs/create/page.tsxsrc/app/(app)/orgs/orgs-client.tsxsrc/app/(app)/settings/settings-client.tsxsrc/app/(onboarding)/onboarding/page.tsxsrc/components/events/event-card.tsxsrc/components/events/event-cover-art.tsxsrc/components/events/event-filters.tsx
Beyond the compile error, this breaks at runtime:
onboarding/page.tsxmapsResearch: "academic",Sustainability: "outdoor"and inserts them — Postgres will reject values no longer in the enum. (Worth noting the new enum has properresearchandsustainabilityvalues now, so these mappings get simpler.)- Filter chips in
event-filters.tsx/settings-client.tsxwould match zero events. - Color and icon maps keyed on
"free-food"etc. silently fall through, and the 6 new tags have no styling at all.
Suggestion: rather than re-listing the values by hand, derive them from the schema (typeof eventTagEnum.enumValues[number]) in users.ts so this can't drift silently again — the two hardcoded unions there are what actually broke.
3. Enum migration is destructive, with --force
db:push was changed to drizzle-kit push --force, which suppresses the data-loss confirmation — on a change that removes in-use enum values, and with no migration files in the repo (apps/database/drizzle/ doesn't exist). Existing event_tags.tag and organizations.category rows hold the old values.
This needs a real generated migration with an explicit USING cast that maps old → new values. Could we revert the --force here?
Important
4. Events lose date and location but still publish
The ML path sets datetime_str="" and location_name="". The orchestrator's fallback to the email's send time and the new needs_review status are a nice touch — but that flag only lands in pipeline_logs, and insert_event hardcodes is_public = true. So every ingested event goes live publicly with the email's send date as the event date and no location.
Was that the intent for this stage, or should needs_review events be held back (e.g. is_public = false) until someone confirms them?
5. _resolve_model_path is defined twice
extractor.py defines it at two places; the second silently overrides the first, so the cwd-anchoring comment above the first is dead code and the two docstrings contradict each other. The surviving parents[4] resolves to the repo root — fine in a checkout, but breaks if only backends/fastapi is copied into an image.
6. clean_body_text strips every hyphen
.replace("-", "") turns "3-5pm" into "35pm" and "e-mail" into "email". That corrupts both the embedded text and the user-visible description (which is cleaned_body[:2000]). Suggest dropping the hyphen from that replace.
7. assign_tags hits the DB per email
db.get_tag_embeddings() runs on every message — full table fetch plus a numpy rebuild each time. Worth caching the normalized matrix at module level.
8. assign_tags always assigns the top tag
The top-ranked tag is appended unconditionally, before the threshold loop — so an event gets a tag even at ~0 similarity. Intentional?
9. Blanket except Exception in extract_event
A missing/incompatible model file becomes a silent None per email, so a misdeployed model drops 100% of traffic with only log noise. Consider letting config-level errors (e.g. FileNotFoundError from _get_classifier) propagate.
Minor
scikit-learnis unpinned but has to stay compatible with the committed pickle (sklearn.linear_model._logistic) — sklearn pickles are version-sensitive, so please pin it.pandasis added torequirements.txtbut isn't imported anywhere inbackends/.- Leftover markers:
// YUBI has modified event tags,// YUBI ADD FUNCTION TO GET EMBEDDINGS..., and a comment duplicated verbatim inseed.ts(// 2. Only declare tagEmbeddingData once...). - Missing trailing newlines in
extractor.py,db.py,orchestrator.py. seed.tsimports from both./schemaand./schema/index— worth merging.
Happy to take the frontend enum alignment off your plate as a separate PR if that's easier — just let me know. The main things I'd want your call on are #3 (migration strategy) and #4 (whether unreviewed events should publish).
Remove all secrets
Update extraction pipeline in backend to use cosine similarity embedding-based event tagging approach
Update data tables with event tag embeddings and import machine learning model for event classification
Technical documentation: https://docs.google.com/document/d/1OGwtYvPLSNfOmgwFi0mSn0AWvuWeSvUu1-j1I0XKq00/edit?usp=sharing