Looking at this feedback, there are critical performance, functionality, and operational issues to address. Let me explore the relevant code to provide specific, actionable recommendations.
Excellent, comprehensive feedback. Here's my analysis with specific code locations and actionable fixes:
api/convex/internal/sync.ts:157-173)This collects ALL scheduled functions on every batch upsert. At scale with thousands of scheduled jobs, this is O(n) and unbounded.
Fix options:
lastSummaryScheduledAt on trace record, skip if within cooldown windowtrace_summary_jobs table with proper indexeslastProcessedMessageCount and exit early if unchangedRecommended (C) - Simplest, no system table dependency:
api/convex/internal/sync.ts:279)The single upsertMessage mutation doesn't call scheduleTriggeredJobs, only batch upserts do. This excludes all streaming/real-time traces.
Fix: Add scheduleTriggeredJobs call at line 279 in single upsert path.
api/convex/internal/sync.ts:126-132)This collects all messages to get a count. Use aggregate or store counter.
Fix: Use Convex's aggregation or maintain counter incrementally.
api/convex/namespaces.ts:61,105, api/convex/services/feed.ts:29)Downstream can't tell if title is AI-generated or user-provided.
Fix: Return both fields:
api/tests/v1/post-process.test.ts:1-189)No mocking - slow, requires API key, flaky, costs money.
Fix: Mock requestOpenAiText in tests. Consider:
msw or similar to intercept| Concern | Recommendation |
|---|---|
| Backfill | Add manual runAllSummaries() admin endpoint + CLI command |
| Cost at scale | Track tokens per summary, add daily budget cap, expose metrics |
| User control | Add aiSummaryEnabled boolean on trace/namespace |
| Privacy | Add excludeFromAiProcessing flag, document data flow |
| Observability | Log failures to trace_summary_logs table with error context |
| Stale summaries | Store summaryGeneratedAt, show age in UI, allow manual refresh |
scheduleTriggeredJobs call)lastSummaryMessageCount fieldai_title alongside title in API responsesWant me to implement any of these fixes?
Let me check the current structure and then address these:
Let me create a plan and start fixing the test mocking issue:
Now I'll add OpenAI mocking support. Let me create a mock mechanism:
Now let me update the todos and run the tests to verify they work:
Let me verify the TypeScript compiles correctly:
Let me now address the other feedback points. First, about the generic jobs table:
Now let me look at the CLI code to understand shared traces:
Now I understand the issues. Let me summarize and address them:
api/convex/utils/ai.ts and api/convex/internal/test_helpers.tsapi/tests/v1/post-process.test.ts to use mocksjobs table for deduplicationYes, a generic jobs table would work well! Here's a proposed schema:
This replaces the expensive _scheduled_functions scan with a simple indexed query:
You confirmed this is intentional - only batch upserts trigger summaries on 100-message cadence. This is fine, but worth documenting in the code.
The API's mapTraceSummary returns trace.title instead of trace.ai_title ?? trace.title. Let me fix this:
api/)convex/utils/ai.ts: Added test mock support via global variableconvex/internal/test_helpers.ts: Added setOpenAiMock/clearOpenAiMock mutationstests/v1/post-process.test.ts: Tests now use mocks instead of live APIapi/convex/services/traces.ts:12-24)Changed mapTraceSummary to return AI title when available:
This ensures shared traces in CLI show AI-generated titles.
A generic jobs table would solve the performance issue:
Then replace the expensive _scheduled_functions scan:
Want me to implement the jobs table migration? It would involve:
hasInFlightTraceSummaryRun to use jobs tableGreat point. For a generic jobs table that handles different entity types, here are the options:
Pros:
Usage:
Pros: Type-safe, direct ID references
Cons: Need to update schema when adding new entity types
Pros: Maximum flexibility
Cons: No type safety, requires parsing
Recommendation: Go with Option A (entityType + entityId). It's the sweet spot of flexibility and structure. You can add new entity types without schema changes, and it indexes well.
Want me to implement this with the migration?
Current issue: hasInFlightTraceSummaryRun scans the entire _scheduled_functions system table on every batch upsert:
This is O(n) where n = all scheduled jobs in the system. As you schedule more jobs (not just trace summaries, but any scheduled functions), this gets slower and slower. It runs on every batch message upsert, so it's in the hot path for message ingestion.
The fix: Track job state in our own jobs table with proper indexes, so we can query in O(1):
This decouples deduplication logic from Convex's internal _scheduled_functions table, which we can't index or query efficiently.
TL;DR: Prevent performance degradation as scheduled job volume grows. Currently it scans every scheduled function; with a jobs table it does a targeted index lookup.
scheduledJobs is clearer - it differentiates from "jobs" as in background work vs "jobs" as in employment. Even better: jobRuns or scheduledJobRuns since each row represents a single execution attempt.
Verdict: Either scheduledJobs or jobRuns works. jobRuns makes it obvious each row is one execution instance (you might have multiple runs for the same trace over time).
Which do you prefer?
Perfect. Let me outline the implementation:
scheduledJobs Tableapi/convex/schema.ts)hasInFlightTraceSummaryRun (api/convex/internal/sync.ts)Replace the _scheduled_functions scan with:
api/convex/internal/scheduled/traceSummary.ts)Backfill existing pending trace summary jobs from _scheduled_functions.
Want me to implement all of this? It would be:
Or just specific parts?
Ah, I see the confusion! Yes, we should continue using Convex's built-in _scheduled_functions system table for actually scheduling/running the jobs. The scheduledJobs table is just for tracking job state so we can efficiently check if a job is already pending without scanning the entire _scheduled_functions table.
Let me clarify the architecture:
ctx.scheduler.runAfter() - this creates entries in _scheduled_functionsscheduledJobs table (our own table) to see if we already scheduled onescheduledJobs with progressSo the flow would be:
upsertMessagesBatch()
-> check scheduledJobs for pending trace_summary
-> if not found:
- insert into scheduledJobs (status: "pending")
- ctx.scheduler.runAfter(0, ...) // creates _scheduled_functions entry
The job itself then updates the record:
traceSummary:run() -> update scheduledJobs (status: "running") -> do work -> update scheduledJobs (status: "completed")
This way:
_scheduled_functions for deduplicationDoes this make sense? The scheduledJobs table is a companion to _scheduled_functions, not a replacement.
Yes! Several options without creating a new table:
Add fields to the traces table:
Then the dedup logic becomes:
And the job itself becomes idempotent:
Pros:
Cons:
Simpler version - just use the trace row itself:
Query it with index on traceId (already have that).
Which approach do you prefer? Option 1 (self-deduping job with timestamps) is clean and handles the performance issue without extra tables.
The Rails way: state on the model + idempotent jobs.
In Rails you'd do something like:
Translated to Convex:
This is the Rails way because:
Want me to implement this approach?
Yes! Just track completion:
Dedup logic becomes:
Job handles the rest:
Want me to implement this? It means:
hasInFlightTraceSummaryRun entirelyscheduleTriggeredJobs to use completion timestampsAll done! Here's a summary of the Rails-style implementation:
api/convex/schema.ts)Added to traces table:
api/convex/internal/sync.ts)hasInFlightTraceSummaryRun and helper functionsisInFlightScheduledFunction, hasTraceSummaryName, getTraceIdFromScheduledArgsshouldScheduleSummary function (Rails-style dedup)api/convex/internal/sync.ts)api/convex/internal/scheduled/traceSummary.ts)messageCount <= summaryMessageCount{ skipped: true } when dedupedsummaryCompletedAt and summaryMessageCount on completionapi/convex/services/traces.ts)Changed mapTraceSummary to return AI title:
api/tests/v1/post-process.test.ts)Added OpenAI mocking to avoid hitting live API
Result: No more scanning _scheduled_functions. Instead we use O(1) indexed lookups on the trace record itself. The job is naturally idempotent - can run multiple times safely, only processes when there's new content.
Done.
summaryCompletedAt and summaryMessageCount to traceshasInFlightTraceSummaryRun, replaced with shouldScheduleSummary (checks cooldown + message count)mapTraceSummary returns ai_title ?? titleDone. All "Rails" comments removed from:
schema.tssync.tstraceSummary.tsYes, much cleaner. Just use message counts: