Conversation
jhodapp
left a comment
There was a problem hiding this comment.
Looking great, here's some initial feedback. I did get through the entire thing, but I'm guessing we'll want to discuss some things further that I suggested.
|
|
||
| ## Implementation Steps | ||
|
|
||
| ### Step 1: Database Migrations + SeaORM Entities |
There was a problem hiding this comment.
@calebbourg I've got a proposal to track cost metrics that I did some brainstorming on with Claude. Tracking costs for this is really important since it could get away from us pretty quickly, so I'd like to add it into this work for milestone 2.
I also have an idea for refactoring the provider field in the new api_credentials table.
Proposal: Add cost observability tracking + provider enum hierarchy (Step 1)
Provider Enum Refactor
The existing Provider enum (Google, Zoom) currently covers meeting platforms. With Recall.ai, AssemblyAI, and LLM Gateway entering the picture, I'd like to split this into a hierarchy:
MeetingProvider(PostgreSQL enum meeting_provider) —Google,Zoom- Used by:
coaching_sessions.provider
- Used by:
PipelineProvider(PostgreSQL enum pipeline_provider) —RecallAi,AssemblyAi,LlmGateway- Used by:
api_credentials.provider,cost_pricing_config.provider,platform_cost_metrics.provider - Named "pipeline" because these services form the record → transcribe → analyze chain
- Used by:
Provider(Rust-only, not in DB) — parent enum wrappingMeeting(MeetingProvider)andPipeline(PipelineProvider)for domain logic that needs to handle any provider generically
Migration renames the existingproviderPG enum tomeeting_providerand createspipeline_provider.
Cost Observability Tables
meeting_recordings.duration_seconds already gives us bot-minutes per session. I'd like to layer lightweight cost tracking on top so we can monitor what each pipeline service costs us in aggregate.
New table: cost_pricing_config
id UUID PK DEFAULT gen_random_uuid()
provider pipeline_provider NOT NULL -- which service: recall_ai, assembly_ai, llm_gateway
metric_name cost_metric NOT NULL -- ENUM: bot_minutes, transcription_hours, llm_tokens
unit cost_unit NOT NULL -- ENUM: minutes, hours, tokens
cost_per_unit_low DOUBLE PRECISION NOT NULL -- lowest known/quoted rate per unit (optimistic estimate)
cost_per_unit_high DOUBLE PRECISION NOT NULL -- highest known/quoted rate per unit (conservative estimate)
cost_per_unit_avg DOUBLE PRECISION NOT NULL -- negotiated or midpoint rate — the "real" estimate
effective_from TIMESTAMPTZ NOT NULL DEFAULT NOW() -- when this rate took effect; new row per rate change, old rows preserved
UNIQUE(provider, metric_name, effective_from)New table: platform_cost_metrics
id UUID PK DEFAULT gen_random_uuid()
provider pipeline_provider NOT NULL
metric_name cost_metric NOT NULL
coaching_session_id UUID FK → coaching_sessions(id) ON DELETE SET NULL -- ties cost to a session for per-session breakdowns
source_record_id UUID NOT NULL -- logical FK to the record that generated this cost
-- (meeting_recordings.id, transcriptions.id, etc.)
-- not a DB-level FK since it can point to different tables
cost_low DOUBLE PRECISION NOT NULL -- quantity * cost_per_unit_low at time of recording
cost_high DOUBLE PRECISION NOT NULL -- quantity * cost_per_unit_high at time of recording
cost_avg DOUBLE PRECISION NOT NULL -- quantity * cost_per_unit_avg at time of recording
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
INDEX(provider, created_at) -- time-range rollups ("what did Recall.ai cost this month?")
INDEX(coaching_session_id) -- per-session cost breakdown queriesNew enums (with ALTER TYPE ... OWNER TO refactor per CLAUDE.md):
cost_metric: bot_minutes, transcription_hours, llm_tokens
cost_unit: minutes, hours, tokens
How it works:
cost_pricing_configstores per-unit rates by provider — updateable without a redeploy, with effective_from for rate history- When a recording or transcription completes, the webhook handler looks up the current rate from
cost_pricing_configand writes aplatform_cost_metricsrow - Raw quantities (minutes, hours, tokens) stay in their source tables — no duplication. The cost table only stores what it uniquely knows: the cost estimates at the rates in effect at write time
- Provider-generic design means adding cost tracking for new pipeline services is just new config rows + new metric rows, no schema change
Gives us immediately:
- Monthly cost rollups by provider
- Per-session cost breakdowns
- Rate change history without losing old pricing data
- Ability to spot cost anomalies early
| - Recall.ai webhooks use Svix HMAC-SHA256 (needs Svix-specific validator added to `meeting-auth`) | ||
| - AssemblyAI webhooks use custom header auth (not HMAC) | ||
| - Providers constructed inline at call site — no AppState provider fields needed | ||
| - `audio_url` is internal-only — not serialized to API clients (`#[serde(skip_serializing)]`) |
There was a problem hiding this comment.
As far as I understand, recall.ai returns an audio_url field that is a link to the audio of the file. It's saying that we don't serialize this in the responses to the FE so the user never sees it at the moment
| - Providers constructed inline at call site — no AppState provider fields needed | ||
| - `audio_url` is internal-only — not serialized to API clients (`#[serde(skip_serializing)]`) | ||
| - Transcript content lives exclusively in `transcript_segments`; `transcriptions` stores only metadata | ||
| - Speaker labels resolved to real user names via the coaching relationship at segment creation time |
There was a problem hiding this comment.
Is it possible to pass into/out of the user's UUID to recall.ai/assembly.ai so it's very easy to correlate the correct utterance segments?
| Implements `meeting_ai::traits::transcription::Provider`. Constructed per-request with user's API key. | ||
|
|
||
| ```rust | ||
| pub struct Provider { |
There was a problem hiding this comment.
I believe that my proposed new Provider enum type makes sense to be added as a provider type in the trait (see longer suggestion comment above).
| - `recall_ai_region: String` — env: `RECALL_AI_REGION`, default `"us"` | ||
| - `recall_ai_webhook_secret: SecretString` — env: `RECALL_AI_WEBHOOK_SECRET` (Svix signing secret) | ||
| - `assembly_ai_webhook_secret: SecretString` — env: `ASSEMBLY_AI_WEBHOOK_SECRET` (custom header value) | ||
| - `webhook_base_url: String` — env: `WEBHOOK_BASE_URL` (e.g., `https://app.refactorcoach.com`) |
There was a problem hiding this comment.
Don't we already have a base URL env var we can reuse? I'm worried about having too many static env vars that we could even use existing ones to calculate new ones from.
| - `recall_ai_webhook_secret: SecretString` — env: `RECALL_AI_WEBHOOK_SECRET` (Svix signing secret) | ||
| - `assembly_ai_webhook_secret: SecretString` — env: `ASSEMBLY_AI_WEBHOOK_SECRET` (custom header value) | ||
| - `webhook_base_url: String` — env: `WEBHOOK_BASE_URL` (e.g., `https://app.refactorcoach.com`) | ||
| - `assembly_ai_analysis_model: String` — env: `ASSEMBLY_AI_ANALYSIS_MODEL`, default `"claude-sonnet-4-6"` |
There was a problem hiding this comment.
For a coach that wants to select their own model, would this just act as a default model to use but a coach could override this from their Settings page on the frontend?
Description
describe the intent of your changes here
GitHub Issue: [Closes|Fixes|Resolves] #your GitHub issue number here
Changes
Testing Strategy
describe how you or someone else can test and verify the changes
Concerns
describe any concerns that might be worth mentioning or discussing