feat: add comprehensive analytics tracking for key factor creation#4197
feat: add comprehensive analytics tracking for key factor creation#4197SylvainChevalier wants to merge 3 commits intomainfrom
Conversation
Track all key factor creation interactions: - Manual creation vs LLM-generated - "Ask an LLM" button clicks - LLM suggestion acceptance, editing, and rejection - Successful LLM generation with counts by type Analytics events: - addKeyFactor: Enhanced with source (manual/llm_suggestion/mixed) and type breakdown - keyFactorGenerateClicked: When user requests LLM suggestions - keyFactorLLMGenerated: When backend successfully generates suggestions - keyFactorLLMSuggestionAccepted: When user accepts a suggestion - keyFactorLLMSuggestionEdited: When user edits a suggestion - keyFactorLLMSuggestionRejected: When user rejects a suggestion Fixes #4194 Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
SylvainChevalier
temporarily deployed
to
testing_env
GitHub Actions
Inactive
SylvainChevalier
temporarily deployed
to
testing_env
GitHub Actions
Inactive
📝 WalkthroughWalkthroughThis PR adds analytics instrumentation across the key-factor lifecycle: generation clicks, LLM suggestion generation/results (counts by type), edits, accepts, rejects, and submission source categorization (manual/llm_suggestion/mixed) in three frontend components. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as KeyFactors UI
participant LLM as LLM Service
participant Analytics as Analytics Service
User->>UI: Click "Generate" / open suggestions
UI->>Analytics: sendAnalyticsEvent(keyFactorGenerateClicked {event_category, force})
UI->>LLM: fetchSuggestions(...)
LLM-->>UI: suggestions (driver/base_rate/news)
UI->>Analytics: sendAnalyticsEvent(keyFactorLLMGenerated {counts_by_type, total})
Note right of UI: User inspects suggestions
User->>UI: Edit / Accept / Reject suggestion
UI->>Analytics: sendAnalyticsEvent(keyFactorLLMSuggestionEdited|Accepted|Rejected {event_category})
UI-->>Analytics: sendAnalyticsEvent(keyFactorLLMSuggestionRejected) before removal (if applicable)
User->>UI: Submit key factors
UI->>Analytics: sendAnalyticsEvent(addKeyFactor {source: manual|llm_suggestion|mixed, submitType, total_count})
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (2)front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SylvainChevalier
temporarily deployed
to
Preview
GitHub Actions
Inactive
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
|
@claude also keep track of attempted generations when the user doesn't even click the button. I think the front end shows a discreet loading animation while it's working. |
|
Claude finished @SylvainChevalier's task in 2m 43s —— View job Completed: Tracking automatic LLM generation attempts ✓
What I didAdded tracking for automatic LLM generation attempts that happen in the background before users click any button. New analytics event:
This tracks the "discreet loading animation" phase you mentioned, where the spinner appears on the "Add Key Factor" button while the system works in the background. Changes made
--- | |
Add keyFactorLLMGenerationAttempted analytics event to track when automatic background LLM generation is triggered for key factors. This captures attempts before the user clicks any button, during the discreet loading animation phase. Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@front_end/src/app/`(main)/questions/[id]/components/key_factors/hooks.ts:
- Around line 310-327: The source detection (variables submittedDrafts and
suggested) runs before empty drafts are filtered, so hasManualDrafts/suggested
can be wrong and sendAnalyticsEvent may report "mixed" incorrectly; update the
logic to determine source from the actual filtered results (e.g., the arrays
produced after filtering or from writePayloads by checking each payload's
origin), then set source ("manual", "llm_suggestion", "mixed") based on those
filtered arrays or payload flags and call sendAnalyticsEvent with that corrected
source.
🧹 Nitpick comments (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_context.tsx (1)
228-239: Analytics event fires before actual suggestion loading occurs.The analytics event
keyFactorGenerateClickedis tracked correctly when the user clicks the "Ask an LLM" button. The early return on line 229 prevents duplicate tracking during in-flight requests.However, consider whether the early return should be placed after the analytics call to track all user click intents, even if a request is already in progress. This would provide more accurate data on user engagement patterns.
Optional: Track clicks even during loading
loadSuggestions: (force?: boolean) => { + // Track when user clicks "Ask an LLM" button + sendAnalyticsEvent("keyFactorGenerateClicked", { + event_category: commentId ? "fromComment" : "fromList", + force: !!force, + }); + if (isLoadingSuggestedKeyFactors) return; - - // Track when user clicks "Ask an LLM" button - sendAnalyticsEvent("keyFactorGenerateClicked", { - event_category: commentId ? "fromComment" : "fromList", - force: !!force, - }); if (force) reloadSuggestions(); else if (!shouldLoadSuggestions) setShouldLoadSuggestions(true); },front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (1)
221-229: Clarify "Edited" event semantics and consider extracting type detection.The
keyFactorLLMSuggestionEditedevent fires when the user clicks the edit button to start editing, not when they save their edits. This may or may not align with analytics requirements—verify this captures the intended user action.Additionally, the type detection logic is duplicated across three locations (edit, accept, reject handlers). Consider extracting it to reduce repetition.
Extract type detection helper
Add this helper near the top of the component:
const getKeyFactorType = (kf: KeyFactorDraft): "driver" | "base_rate" | "news" => isDriverDraft(kf) ? "driver" : isBaseRateDraft(kf) ? "base_rate" : "news";Then use it in all three locations:
- const keyFactorType = isDriverDraft(kf) - ? "driver" - : isBaseRateDraft(kf) - ? "base_rate" - : "news"; + const keyFactorType = getKeyFactorType(kf);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/hooks.tsfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_context.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (2)
front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)front_end/src/utils/key_factors.ts (3)
isDriverDraft(9-10)isBaseRateDraft(11-12)isNewsDraft(13-14)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_context.tsx (1)
front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (2)
front_end/src/utils/key_factors.ts (2)
isDriverDraft(9-10)isBaseRateDraft(11-12)front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)
🔇 Additional comments (6)
front_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_context.tsx (1)
242-260: LGTM!The addition of
commentIdto the dependency array is correct since it's now referenced in theloadSuggestionscallback for the analytics payload.front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (2)
721-740: Analytics tracks user intent, not outcome.The
keyFactorLLMSuggestionAcceptedevent fires before the async submission completes. If the submission fails (lines 733-738), the event is still recorded even though the acceptance didn't succeed. This is acceptable if the intent is to track user actions rather than outcomes—the actual successful submissions are tracked separately viaaddKeyFactorinhooks.ts.
747-762: LGTM!The reject analytics event is correctly placed before the removal action, ensuring the key factor type can be determined before it's removed from the suggestions list.
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (3)
130-140: LGTM!The
keyFactorLLMGeneratedevent provides useful analytics with counts broken down by type. The multiple.filter()calls are acceptable given the typically small array size.
147-167:⚠️ Potential issue | 🟡 MinorAnalytics event may fire for non-existent comments.
The
keyFactorLLMGenerationAttemptedevent fires at line 156, but if the comment is not found (line 152 returns early), the analytics event is still recorded even though no generation attempt actually occurs.Consider moving the analytics call after the comment existence check:
Proposed fix
useEffect(() => { if (!shouldLoadKeyFactors || !commentId) return; if (questionLinksCheckedRef.current.has(commentId)) return; const comment = comments.find((c) => c.id === commentId); if (!comment) return; // Track automatic generation attempt (before user clicks button) - if (!fetchedOnceRef.current.has(commentId)) { + const isFirstAttempt = !fetchedOnceRef.current.has(commentId); + if (isFirstAttempt) { sendAnalyticsEvent("keyFactorLLMGenerationAttempted", { event_category: "automatic", }); } const ids = extractQuestionNumbersFromText(comment.text || "");Actually, looking more closely, the current placement at lines 154-159 is after the comment existence check at line 152. The control flow is correct—my initial reading was wrong. The analytics only fires if a valid comment exists.
154-159: LGTM!The
keyFactorLLMGenerationAttemptedevent correctly tracks automatic/background generation attempts. The placement after the comment existence check ensures analytics only fires for valid scenarios, and thefetchedOnceRefcheck prevents duplicate tracking.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
|
||
| // Determine if this is a manual creation or from LLM suggestions | ||
| const hasManualDrafts = submittedDrafts.length > 0; | ||
| const hasSuggestedDrafts = suggested.length > 0; |
There was a problem hiding this comment.
Nit: submittedDrafts and suggested are the raw parameters before empty drafts get filtered out (the filtering happens above via finalDrivers, finalBaseRates, etc.). So if a user has an empty manual draft alongside accepted LLM suggestions, this would report "mixed" when it should be "llm_suggestion". Not a big deal for analytics accuracy but worth noting — you could compute source from the filtered writePayloads instead if you want it to match exactly what gets submitted.
| opts?: { showErrors?: boolean } | ||
| ) => { | ||
| // Track edit action | ||
| const keyFactorType = isDriverDraft(kf) |
There was a problem hiding this comment.
Nit: this type-detection ternary appears 3 times (edit, accept, reject). Could extract a small helper:
const getKeyFactorType = (kf: KeyFactorDraft): "driver" | "base_rate" | "news" =>
isDriverDraft(kf) ? "driver" : isBaseRateDraft(kf) ? "base_rate" : "news";
|
@claude address the review comments. |
|
Claude finished @SylvainChevalier's task in 1m 46s —— View job Addressed review comments ✓Tasks:
Changes made1. Fixed source detection bug (hooks.ts:310-336)The source detection logic now counts only the filtered drafts (after empty drafts are removed) instead of the raw Before: const hasManualDrafts = submittedDrafts.length > 0;
const hasSuggestedDrafts = suggested.length > 0;After: const manualCount =
(submitType === "driver"
? driverDrafts.filter((d) => d.driver.text.trim() !== "").length
: 0) +
(submitType === "base_rate" ? baseRateDrafts.length : 0) +
(submitType === "news" ? newsDrafts.length : 0);
const suggestedCount =
(submitType === "driver"
? suggestedDriverDrafts.filter((d) => d.driver.text.trim() !== "").length
: 0) +
(submitType === "base_rate" ? suggestedBaseRateDrafts.length : 0) +
(submitType === "news" ? suggestedNewsDrafts.length : 0);2. Extracted type-detection helper (key_factors_add_in_comment_llm_suggestions.tsx:87-91)Created a const getKeyFactorType = (
kf: KeyFactorDraft
): "driver" | "base_rate" | "news" =>
isDriverDraft(kf) ? "driver" : isBaseRateDraft(kf) ? "base_rate" : "news";This helper is now used in all three analytics tracking locations (edit, accept, reject handlers) instead of repeating the ternary expression. --- | |
- Fix source detection bug where empty manual drafts could cause misclassification - Now counts only filtered (non-empty) drafts to accurately determine source - Extract duplicated key factor type detection into getKeyFactorType helper - Reduces code duplication across edit, accept, and reject handlers Co-authored-by: Sylvain <SylvainChevalier@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (1)
224-247:⚠️ Potential issue | 🟠 MajorAnalytics events fire incorrectly on failed accept → edit fallback path.
Two problems with the current instrumentation:
keyFactorLLMSuggestionAcceptedfires before the server call succeeds (line 728). IfaddSingleSuggestedKeyFactorreturns errors (line 733), the accept didn't actually happen, yet the event was already sent.
handleEditunconditionally fireskeyFactorLLMSuggestionEdited(line 231). When accept fails and falls back tohandleEdit(kf, keyFactorIndex, { showErrors: true })at line 734, a spurious "Edited" event is emitted for what was really a failed accept.The net result: a single failed accept attempt logs both "Accepted" and "Edited" — polluting both metrics.
Proposed fix: move Accepted event after success, skip Edited on fallback
For the accept handler, move the event after the success check:
onClick={async () => { - // Track accept action - const keyFactorType = getKeyFactorType(kf); - sendAnalyticsEvent("keyFactorLLMSuggestionAccepted", { - event_category: keyFactorType, - }); - const res = await addSingleSuggestedKeyFactor(kf); if (!res || ("errors" in res && res.errors)) { handleEdit(kf, keyFactorIndex, { showErrors: true, }); return; } + // Track accept action only on success + const keyFactorType = getKeyFactorType(kf); + sendAnalyticsEvent("keyFactorLLMSuggestionAccepted", { + event_category: keyFactorType, + }); removeKeyFactorAt(keyFactorIndex); }}For
handleEdit, add an option to suppress the analytics event when called from the fallback path:const handleEdit = ( kf: KeyFactorDraft, idx: number, - opts?: { showErrors?: boolean } + opts?: { showErrors?: boolean; skipAnalytics?: boolean } ) => { - // Track edit action - const keyFactorType = getKeyFactorType(kf); - sendAnalyticsEvent("keyFactorLLMSuggestionEdited", { - event_category: keyFactorType, - }); + if (!opts?.skipAnalytics) { + const keyFactorType = getKeyFactorType(kf); + sendAnalyticsEvent("keyFactorLLMSuggestionEdited", { + event_category: keyFactorType, + }); + }Then in the accept fallback:
- handleEdit(kf, keyFactorIndex, { showErrors: true }); + handleEdit(kf, keyFactorIndex, { showErrors: true, skipAnalytics: true });Also applies to: 725-740
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (2)
147-168:⚠️ Potential issue | 🟡 Minor
keyFactorLLMGenerationAttemptedcan fire multiple times for the same comment.When
extractQuestionNumbersFromTextfinds IDs (line 161-162 falls through),questionLinksCheckedRefis only set inside the asyncrun()(lines 190/198). Ifcommentschanges whilerun()is in-flight, this effect re-enters, passes thequestionLinksCheckedRefguard (line 149), and thefetchedOnceRefguard (line 155) also passes becausefetchSuggestionshasn't completed yet — resulting in duplicate "attempted" events.A simple fix: track "attempted" separately, or set
questionLinksCheckedRefsynchronously before entering the async path.Proposed fix: set questionLinksCheckedRef before async work
void run(); + questionLinksCheckedRef.current.add(commentId); return () => { cancelled = true; };Alternatively, use a dedicated ref for the analytics event:
+ const attemptTrackedRef = useRef<Set<number>>(new Set()); // ... - if (!fetchedOnceRef.current.has(commentId)) { + if (!fetchedOnceRef.current.has(commentId) && !attemptTrackedRef.current.has(commentId)) { + attemptTrackedRef.current.add(commentId); sendAnalyticsEvent("keyFactorLLMGenerationAttempted", { event_category: "automatic", }); }
358-384:⚠️ Potential issue | 🟡 Minor
addSingleSuggestedKeyFactoralways reportssource: "manual"in theaddKeyFactorevent.This method passes the draft as a
submittedDraftwith an emptysuggestedarray (line 377:onSubmit([draft], [], submitType)), so the source detection inonSubmitalways computesmanualCount > 0, suggestedCount === 0→source = "manual". Even when this function is called from the LLM suggestion accept flow, theaddKeyFactorevent will saysource: "manual".The separate
keyFactorLLMSuggestionAcceptedevent covers this case, but if you intendaddKeyFactor.sourceto be a reliable indicator of origin, you'd need to either pass the draft assuggestedor add a parameter to override the source.Possible fix: pass draft as suggested when it comes from LLM
- const res = await onSubmit([draft], [], submitType); + const res = await onSubmit([], [draft], submitType);Or add an explicit
sourceoverride parameter toonSubmit.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts
🧰 Additional context used
🧬 Code graph analysis (2)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (3)
front_end/src/types/key_factors.ts (1)
KeyFactorDraft(49-49)front_end/src/utils/key_factors.ts (2)
isDriverDraft(9-10)isBaseRateDraft(11-12)front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (2)
front_end/src/utils/analytics.ts (1)
sendAnalyticsEvent(11-17)front_end/src/utils/key_factors.ts (3)
isDriverDraft(9-10)isBaseRateDraft(11-12)isNewsDraft(13-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Docker Image / Build Docker Image
- GitHub Check: Frontend Checks
- GitHub Check: Backend Checks
- GitHub Check: integration-tests
🔇 Additional comments (4)
front_end/src/app/(main)/questions/[id]/components/key_factors/add_in_comment/key_factors_add_in_comment_llm_suggestions.tsx (2)
87-93: LGTM — clean helper for type determination.The ternary chain is correct given the exhaustive union
KeyFactorDraft = DriverDraft | BaseRateDraft | NewsDraft. Note that a similar pattern exists inhooks.ts(addSingleSuggestedKeyFactor, lines 365-373). If more call sites appear, consider extracting this to a shared utility in@/utils/key_factors.ts.
749-757: LGTM — reject tracking is straightforward.The event fires synchronously before removal, and there's no async fallback path that could cause double-counting.
front_end/src/app/(main)/questions/[id]/components/key_factors/hooks.ts (2)
130-140: LGTM — well-placed generation success event with useful breakdowns.The event fires only after filtering and hydration are complete, and includes granular counts by type. Good instrumentation.
310-339: Source detection logic looks correct after the fix.The
manualCount/suggestedCountnow properly use filtered arrays, consistent withfinalDriversfiltering at line 262. The source classification (manual/llm_suggestion/mixed) is sound.One observation:
sendAnalyticsEvent("addKeyFactor", ...)at line 334 fires before the error check at line 341, so failed submissions are also tracked. This is pre-existing behavior not introduced by this PR, but worth noting for future consideration if you want analytics to reflect only successful submissions.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Track all key factor creation interactions:
Analytics events:
addKeyFactor: Enhanced with source (manual/llm_suggestion/mixed) and type breakdownkeyFactorGenerateClicked: When user requests LLM suggestionskeyFactorLLMGenerationAttempted: When automatic LLM generation starts (background)keyFactorLLMGenerated: When backend successfully generates suggestionskeyFactorLLMSuggestionAccepted: When user accepts a suggestionkeyFactorLLMSuggestionEdited: When user edits a suggestionkeyFactorLLMSuggestionRejected: When user rejects a suggestionFixes #4194
Generated with Claude Code
Summary by CodeRabbit