chore: trace funnels bugfixes/improvements

[ahmadshaheer]

📄 Summary

---

✅ Changes

• Bug fix: Add missing refetch query for funnel steps overview on refresh action
• Chore: temporarily hide latency pointer as it

---

🏷️ Required: Add Relevant Labels

⚠️ Manually add appropriate labels in the PR sidebar
Please select one or more labels (as applicable):

ex:

• frontend
• backend
• devops
• bug
• enhancement
• ui
• test

---

👥 Reviewers

Tag the relevant teams for review:

• frontend / backend / devops

---

🧪 How to Test

1. ...
2. ...
3. ...

---

🔍 Related Issues

Closes #

---

📸 Screenshots / Screen Recording (if applicable / mandatory for UI related changes)

---

📋 Checklist

• Dev Review
• Test cases added (Unit/ Integration / E2E)
• Manually tested the changes

---

👀 Notes for Reviewers

---

Important

Improves funnel tracing by adding refetch for funnel steps and hiding latency pointer, with UI and hook enhancements for better state management and user feedback.

• Behavior:
  • Adds refetch query for funnel steps overview on refresh in useFunnelConfiguration.tsx.
  • Temporarily hides latency pointer in FunnelStep.tsx and FunnelStep.styles.scss due to unresolved issues.
• UI Components:
  • Updates AddSpanToFunnelModal.tsx to handle save and discard actions with new state management.
  • Modifies FunnelConfiguration.tsx to integrate new funnel step handling logic.
  • Adjusts styles in AddSpanToFunnelModal.styles.scss and FunnelConfiguration.styles.scss for better UI consistency.
• Hooks:
  • Enhances useFunnelConfiguration.tsx to support manual and auto-save modes with notifications.
  • Refines useLocalStorage.ts to stabilize default values and prevent unnecessary re-renders.

^{This description was created by for c6a24d1. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 3fc0577 in 1 minute and 36 seconds. Click for details.

• Reviewed 83 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/pages/TracesFunnels/FunnelContext.tsx:189

• Draft comment:
  Added missing refetch for GET_FUNNEL_STEPS_OVERVIEW in handleRunFunnel. Ensure that the 'selectedTime' (and any other URL-based params) are correctly captured so that the API call doesn’t receive empty parameters.
• 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.

2. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelStep.tsx:200

• Draft comment:
  Typo: There's a stray '*/}' appended to the closing div on line 200. It appears that an extra comment closing token was introduced here. If this was not intentional, please remove it.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

Workflow ID: wflow_6mN3ZUWwwCG2EOiS

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed ba40f9a in 52 seconds. Click for details.

• Reviewed 15 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/pages/TracesFunnels/FunnelContext.tsx:170

• Draft comment:
  Ensure that the FunnelStepData type is updated to include the new 'filters' field to avoid potential type mismatches.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the author to ensure that a type is updated to include a new field. It is not a direct suggestion or a specific request for a test or refactor. It falls under the rule of not asking the author to ensure something is done, which is not allowed.

2. frontend/src/pages/TracesFunnels/FunnelContext.tsx:173

• Draft comment:
  Verify that the default empty 'filters' ({ items: [], op: 'AND' }) aligns with backend expectations and add tests to ensure URL parameters aren’t sent empty in API calls.
• 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 alignment with backend expectations and to add tests, which violates the rule against asking for confirmation or ensuring behavior is intended. It also suggests adding tests, but in a non-specific way. Therefore, it should be removed.

Workflow ID: wflow_bJJWDN68tRuHaDO4

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 2b2460d in 1 minute and 19 seconds. Click for details.

• Reviewed 61 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/hooks/useLocalStorage.ts:25

• Draft comment:
  Comparing the initialValue using !== may trigger unnecessary ref updates when a new inline function is provided. Confirm that this behavior is intended.
• 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% The comment raises a valid technical point about function identity, but it's phrased as a request to "confirm intention" rather than suggesting a specific fix. The code's behavior with inline functions seems intentional given the comment about handling dynamic values. The !== comparison is actually desired here to detect new function instances. The comment identifies a real JavaScript behavior, but am I being too harsh in rejecting it just because it asks for confirmation? The rules explicitly state not to ask authors to confirm their intention. If there was a real issue, the comment should propose a specific fix instead. Delete the comment because it violates the rule against asking authors to confirm their intentions, and the behavior appears intentional based on the code comments.

2. frontend/src/hooks/useLocalStorage.ts:41

• Draft comment:
  Consider checking for null explicitly (if (item !== null)) instead of a truthy check to correctly handle valid falsy values (e.g., empty strings).
• 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_6sg5KXsEctLpzZ0O

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed af54aa4 in 1 minute and 27 seconds. Click for details.

• Reviewed 40 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/api/traceFunnels/index.ts:198

• Draft comment:
  Removed the p99_latency fallback from FunnelOverviewResponse. Ensure the API always returns a valid 'latency' value, or add defensive handling if missing.
• 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 the author to "ensure" something, which is a form of verification request we should avoid. It's also speculative about potential missing values without evidence of an actual issue. The removal of a TODO and deprecated field is a straightforward change that doesn't require validation comments. Maybe there's a known issue with the API sometimes not returning latency values that the reviewer is aware of? The suggestion for defensive handling could be valuable if there's a history of problems. Without concrete evidence of API issues in the current code, this is still a speculative comment. The TODO removal suggests this was a planned deprecation, not a potential problem area. Delete the comment as it violates our rules against asking for verification and making speculative comments without clear evidence of issues.

2. frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts:51

• Draft comment:
  Removed fallback to p99_latency in the 'P99 Latency' metric. Confirm that 'latency' is always defined in the response to prevent runtime issues.
• 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: 1) It asks for verification rather than pointing out a clear issue 2) It starts with "Confirm that..." which is explicitly discouraged 3) If latency was undefined, it would be caught in testing/build 4) We don't have enough context about the API contract to know if this is actually an issue. The comment could be pointing out a legitimate runtime issue if the API contract isn't guaranteed. The removal of the fallback might indicate there was a reason it existed before. Even if there's a potential issue, the comment isn't actionable - it just asks for verification. If there was a real concern, it should propose a specific code change like adding a null check. The comment should be deleted as it violates multiple review rules by asking for verification without proposing a specific fix, and runtime type issues would be caught in testing.

3. frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts:118

• Draft comment:
  Removed fallback to p99_latency in steps metrics. Verify that upstream data always includes a valid 'latency' value before multiplying 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 verify that upstream data always includes a valid latency value. This falls under the rule of not asking the author to confirm or verify things. Therefore, this comment should be removed.

4. frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts:1

• Draft comment:
  The PR description mentions adding a refetch query for funnel steps overview and fixing URL param issues. These changes aren’t evident in the diff—please ensure these fixes are implemented 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.

Workflow ID: wflow_NJ5llDY8ies1IYU1

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 6309ff9 in 1 minute and 0 seconds. Click for details.

• Reviewed 25 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts:93

• Draft comment:
  Good improvement: using the nullish coalescing operator (?? LatencyOptions.P99) ensures that if steps[stepStart]?.latency_type is undefined, we safely fall back to LatencyOptions.P99. This prevents a potential runtime error when later calling toUpperCase on an undefined value.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining the use of the nullish coalescing operator. It doesn't provide a suggestion, ask for confirmation, or point out a potential issue. According to the rules, purely informative comments should be removed.

2. frontend/src/hooks/TracesFunnels/useFunnelMetrics.ts:119

• Draft comment:
  Removing the optional chaining in the toUpperCase call is now safe since latencyType is guaranteed to have a fallback value. This cleanup makes the intent clearer.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining why a change is safe and how it clarifies intent. It doesn't suggest any action or ask for confirmation, which violates the rule against purely informative comments.

Workflow ID: wflow_ee1uQi0kauCvLCoe

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[aks07]

have a small doubt here. Approving still. Do let me know the logic for handling changing initial value

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 0fc3db7 in 2 minutes and 8 seconds. Click for details.

• Reviewed 324 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.tsx:41

• Draft comment:
  The PR description mentions a bug fix for missing URL parameters (and a refetch query on refresh), but this diff doesn’t show any modifications to the API call logic. Please ensure the API query properly includes URL parameters and triggers a refetch as intended.
• 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 API query includes URL parameters and triggers a refetch, which violates the rule against asking the author to ensure behavior is intended. It also references the PR description, which is not allowed. Therefore, this comment should be removed.

2. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.tsx:91

• Draft comment:
  The PR description also requires tests to verify that URL parameters are not empty when passed to the API call, yet no test cases are added here. Please add tests covering this behavior.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx:24

• Draft comment:
  In the useCallback for handleAddForNewStep, only 'steps.length' is included in the dependency array. For more robust dependency tracking, consider including the full 'steps' array if its content (beyond length) may affect the logic.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/StepsContent.tsx:94

• Draft comment:
  Typo: The button label 'Add for new Step' might be a mistake. Consider changing it to 'Add New Step' for clarity.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_chnBhkqwHefjxVtZ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 897a22c in 56 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/DeleteFunnelStep.tsx:30

• Draft comment:
  The update from 'Delete Funnel' to 'Delete Step' aligns the button label with the modal title and action. This improves clarity.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

2. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/DeleteFunnelStep.tsx:1

• Draft comment:
  The PR description mentions an API params bug and related tests, but this diff doesn’t address any API call logic. Please ensure that the URL parameter bug fix & associated tests are included elsewhere in the PR.
• 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_iLOPq1ISKKYBeLfL

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed f7b973d in 2 minutes and 37 seconds. Click for details.

• Reviewed 62 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.styles.scss:5

• Draft comment:
  The 'justify-content: space-between' declaration is immediately overridden by 'justify-content: flex-start'. Remove the redundant declaration to avoid confusion.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.styles.scss:7

• Draft comment:
  The nested '.funnel-details-page' modifier applies height and overflow styles but its naming might be misleading since it is conditionally applied in the component. Consider renaming it to better reflect its actual usage.
• 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% The comment is about naming, which can be subjective. There's no strong evidence that the current name is problematic - "funnel-details-page" seems reasonably descriptive for a class that handles page-level layout concerns. The comment speculates about conditional usage in the component, but we can't see the component code. The rules say to avoid speculative comments and to delete comments if we need more context. I might be overlooking potential confusion the class name could cause to other developers. The name might truly be misleading in the broader context. Without seeing the component code and how this class is used, we can't definitively say the name is misleading. The rules explicitly state to delete comments if we need more context. Delete the comment. The suggestion is speculative, requires more context to validate, and the current name appears reasonable for its purpose.

3. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.tsx:37

• Draft comment:
  The PR description mentions a fix for API call parameters (refetch query for funnel steps overview) but no changes related to API or parameter handling are visible in this diff. Please ensure the API-related changes are included or properly referenced.
• 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.

4. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.tsx:29

• Draft comment:
  The PR description references 'temporarily hide latency pointer', yet no related code changes are present. Please clarify if this change is handled elsewhere or update the PR description.
• 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_8Z1esO7ja35aReCI

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed e14254f in 1 minute and 0 seconds. Click for details.

• Reviewed 56 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/container/TraceWaterfall/AddSpanToFunnelModal/AddSpanToFunnelModal.tsx:144

• Draft comment:
  Resetting unsaved state (isUnsavedChanges) and triggerSave on 'handleBack' appears intentional. Confirm this reset aligns with the expected auto-save/discard flow without losing any pending changes.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. frontend/src/hooks/TracesFunnels/useFunnelConfiguration.tsx:85

• Draft comment:
  Removal of the debug console.log from the funnel steps comparison is appropriate. Ensure no other debugging logs remain in production code.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

3. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.styles.scss:14

• Draft comment:
  The change from 'justify-content: space-between' to 'flex-start' in both __steps-wrapper and __header may affect layout spacing. Verify that these style adjustments meet design specifications.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_wMlM7f2vRqxZMrti

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 115c480 in 1 minute and 7 seconds. Click for details.

• Reviewed 85 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/api/features/getFeatureFlags.ts:5

• Draft comment:
  Consider adding error handling (or a catch block) in getFeaturesFlags to manage API errors and unexpected response shapes.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

2. frontend/src/hooks/useGetFeatureFlag.tsx:1

• Draft comment:
  The diff does not show any modifications addressing URL query parameters for the /traces endpoint as mentioned in the issue description. Verify that the missing params problem is fully resolved.
• 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 verify that a problem is resolved, which violates the rule against asking for confirmation or verification. It does not provide a specific suggestion or point out a clear issue in the code.

3. frontend/src/hooks/useGetFeatureFlag.tsx:13

• Draft comment:
  Ensure that the onSuccessHandler is updated to work with the new data shape (an array) since the previous version extracted data.data from the response.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50% None

4. frontend/src/pages/TracesFunnelDetails/components/FunnelConfiguration/FunnelConfiguration.styles.scss:14

• Draft comment:
  Updated header styling improves alignment; please confirm that the spacing and use of design tokens remain consistent with overall UI guidelines.
• Reason this comment was not posted:
  Confidence changes required: 10% <= threshold 50% None

Workflow ID: wflow_bGknSUx03EP8I5xK

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[aks07]

Added a comment.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed d9812d1 in 3 minutes and 50 seconds. Click for details.

• Reviewed 96 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. frontend/src/container/TraceWaterfall/AddSpanToFunnelModal/AddSpanToFunnelModal.tsx:205

• Draft comment:
  Avoid using inline styles. Instead of inline style (height: 400), move this styling to a CSS class or use design tokens.
• 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.

2. frontend/src/container/TraceWaterfall/AddSpanToFunnelModal/AddSpanToFunnelModal.tsx:149

• Draft comment:
  Using setTimeout to reset the 'triggerDiscard' flag may lead to potential race conditions. Consider cleaning up the timeout on unmount or exploring a more robust state reset strategy.
• 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% The setTimeout usage here is actually a common pattern for trigger flags. The useEffect in FunnelDetailsView will run when triggerDiscard becomes true, and the 100ms delay before resetting is unlikely to cause issues since the restore operation is synchronous. The cleanup suggestion is technically correct but likely overkill for this simple trigger pattern. I could be underestimating the race condition risk. There could be edge cases where rapid discard triggers cause issues. The risk is very low because: 1) The restore operation is synchronous 2) The component would need to be rapidly unmounting/mounting during that 100ms window to hit issues 3) Even if there was a race, the worst case is just an extra restore operation. The comment should be deleted. While technically correct, the setTimeout usage here is a reasonable implementation choice with minimal practical risk.

3. frontend/src/container/TraceWaterfall/AddSpanToFunnelModal/AddSpanToFunnelModal.tsx:88

• Draft comment:
  The PR description mentions fixing API calls and URL param issues, but this diff only updates the discard mechanism. Please verify that the API call bug (URL params missing) is addressed elsewhere as required.
• 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_5KSiLYj7MksBfBBQ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}