feat: update logEvent to pass eventType and replace segment calls wit…

[YounixM]

…h logEvent

---

Important

Replaces Segment analytics with logEvent function, adding event type and rate limiting support, and removes Segment-related code.

• Behavior:
  • Replaces Segment analytics calls with logEvent in index.tsx and AppLayout/index.tsx.
  • Adds eventType and rateLimited parameters to logEvent in logEvent.ts.
  • Removes Segment initialization and usage from index.html.ejs.
• Code Removal:
  • Deletes useAnalytics hook from useAnalytics.tsx.
  • Removes Segment-related environment variables from webpack.config.js and webpack.config.prod.js.
  • Removes Segment mock from InviteUserFlow.test.tsx.
• Misc:
  • Minor formatting changes in index.html.ejs.

^{This description was created by for e410eeb. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 18d67e1 in 1 minute and 36 seconds

More details

• Looked at 315 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 13 drafted comments based on config settings.

1. frontend/src/AppRoutes/index.tsx:88

• Draft comment:
  Refactored analytics: Removed useAnalytics hook and replaced with logEvent calls. Ensure that the new logEvent calls (with eventType additions) are backward-compatible and that any error handling/logging adequately propagates failures.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  This comment is asking the PR author to ensure backward compatibility and proper error handling, which violates the rule against asking the author to ensure behavior is intended or tested. It doesn't provide a specific suggestion or point out a specific issue.

2. frontend/src/AppRoutes/index.tsx:187

• Draft comment:
  Removed trackPageView call from useEffect dependency. Verify that this removal is intentional and that page view tracking is now handled elsewhere.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. frontend/webpack.config.js:23

• Draft comment:
  SEGMENT_ID environment variable removal: Confirm that completely removing SEGMENT_ID from HtmlWebpackPlugin and DefinePlugin is intentional and that no segment-specific code is left elsewhere.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  This comment violates several rules. It's asking for confirmation of intention ("Confirm that...") and speculating about code elsewhere ("segment-specific code is left elsewhere"). The removal of an environment variable is likely an intentional change that was carefully considered. We don't have access to other files to verify segment-specific code anyway.
  The comment might be raising a valid concern about breaking changes if Segment integration is still needed somewhere in the application.
  Per the rules, we should not make speculative comments or ask for confirmation of intentions. If Segment is still needed, that would be caught during testing or build.
  The comment should be deleted as it violates rules against asking for confirmation and making speculative comments about code outside the current file.

4. frontend/webpack.config.prod.js:26

• Draft comment:
  SEGMENT_ID removal in production config: Ensure that all references to SEGMENT_ID are properly removed and that the new analytics integration doesn’t rely on it.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to ensure that all references to SEGMENT_ID are properly removed and that the new analytics integration doesn’t rely on it. This falls under asking the author to double-check things, which is against the rules.

5. frontend/src/AppRoutes/index.tsx:89

• Draft comment:
  Page view tracking was removed (trackPageView call is gone). Confirm that this is intentional, as it may affect analytics.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
  This seems like a potentially significant change - removing page view tracking could impact analytics data collection. However, the comment is phrased as a verification request ("Confirm that this is intentional") which violates our rules. We should not ask authors to confirm their intentions. Additionally, we don't have enough context to know if page view tracking was moved elsewhere or if this was an intentional architectural change.
  I might be overly concerned about lost functionality that could be implemented elsewhere in the codebase. The author may have deliberately consolidated analytics into a different system.
  While the change seems significant, our rules clearly state we should not ask authors to confirm their intentions or make speculative comments. We don't have strong evidence that this is definitely an issue.
  Delete the comment since it violates our rules about asking for confirmation and making speculative comments. Without more context, we can't be certain this is actually an issue.

6. frontend/src/AppRoutes/index.tsx:63

• Draft comment:
  Previously payloads were sanitized (using pickBy) before calling analytics. The new implementation sends the raw payload. Please verify that sending unsanitized payloads won’t introduce unexpected undefined or null values.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the PR author to verify that sending unsanitized payloads won't introduce unexpected undefined or null values. This falls under asking the author to ensure the behavior is intended, which is against the rules. Therefore, this comment should be removed.

7. frontend/src/api/common/logEvent.ts:10

• Draft comment:
  The logEvent function now accepts an optional eventType with a default 'track'. Confirm that the backend endpoint '/event' supports this format and that no sensitive data is sent inadvertently.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

8. frontend/webpack.config.js:23

• Draft comment:
  SEGMENT_ID was removed from HtmlWebpackPlugin options. Ensure that no code references SEGMENT_ID anymore and the analytics vendor switch is fully complete.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  The comment is asking the author to ensure that no code references SEGMENT_ID anymore, which is a form of asking for confirmation or verification. This violates the rule against asking the PR author to confirm or ensure things. Therefore, this comment should not be approved.

9. frontend/webpack.config.prod.js:26

• Draft comment:
  SEGMENT_ID has also been removed in the production config. Confirm that all references to Segment analytics are consolidated in favor of the new logEvent approach.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  This comment asks the author to "confirm" something, which violates the rule against asking authors to verify things. It's also speculative - we can't see the actual analytics implementation from this webpack config file alone. The comment requires knowledge of other files to verify if Segment has been fully removed.
  Maybe this is an important architectural change that needs verification to prevent analytics data loss. The removal of Segment could have wide-ranging implications.
  While analytics are important, the rules clearly state not to make speculative comments or ask authors to verify things. If there were remaining Segment references that needed changing, those should be commented on directly where they appear.
  This comment should be deleted as it asks for verification and requires cross-file context that we don't have.

10. frontend/src/AppRoutes/index.tsx:259

• Draft comment:
  Typographical error: In the comment starting at line 256, 'if the required calls fails then return a something went wrong error' should be corrected to something like 'if the required calls fail then return a 'something went wrong' error' to improve clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. frontend/src/AppRoutes/index.tsx:257

• Draft comment:
  Typographical error: The comment 'move to indefinitive loading' should use 'indefinite' instead of 'indefinitive' for correct English.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. frontend/src/container/OnboardingContainer/__test__/InviteUserFlow.test.tsx:32

• Draft comment:
  Minor grammatical issue: Consider changing '// Check all the option present' to '// Check all the options present' for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. frontend/webpack.config.js:61

• Draft comment:
  Typo: 'BUNDLE_ANALYSER' might be intended to be 'BUNDLE_ANALYZER' to maintain consistency with common spelling conventions. Consider correcting this for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_G9E3kzzEd3CPM1fU

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on e410eeb in 1 minute and 38 seconds

More details

• Looked at 45 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. frontend/src/api/common/logEvent.ts:18

• Draft comment:
  Consider using the nullish coalescing operator (rateLimited ?? false) to ensure that an explicit false value isn't overridden. This makes the intent clearer.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
  While technically correct that ?? would handle explicit false values differently than ||, in this case we're dealing with a boolean parameter where the distinction is minimal. The current code works correctly for the common cases - undefined becomes false, true stays true. The only edge case is if someone explicitly passes null, which is unlikely given the TypeScript type. This feels like a nitpick that doesn't meaningfully improve the code.
  The comment is technically accurate - ?? would handle null differently than ||. And using ?? could be considered slightly more precise.
  However, for a boolean parameter, the distinction between || and ?? is minimal and unlikely to cause real issues. The current code handles all reasonable use cases correctly.
  This comment should be deleted as it suggests a change that provides minimal benefit and doesn't meaningfully improve code quality or prevent bugs.

2. frontend/src/container/AppLayout/index.tsx:393

• Draft comment:
  Ensure the attribute keys (durationMs, thresholdMs) and the explicit eventType 'track' with rateLimited flag align with backend expectations and maintain consistency.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  This comment is asking the PR author to ensure that certain attributes align with backend expectations. This falls under the rule of not asking the author to ensure behavior is intended or to double-check things. Therefore, this comment should be removed.

3. frontend/src/container/AppLayout/index.tsx:393

• Draft comment:
  Refactoring the event attributes to use numeric values (e.g. 'durationMs' and 'thresholdMs') is a good move. Please ensure that the backend is updated to expect these numeric fields.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
  This comment is asking the PR author to ensure that the backend is updated, which violates the rule against asking for confirmation or ensuring behavior. It doesn't provide a specific code suggestion or ask for a specific test to be written.

Workflow ID: wflow_HgdSnO4nk4XIRFu2

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.