Sort scenarios in comparisons consistently wherever displayed#178
Sort scenarios in comparisons consistently wherever displayed#178david-mears-2 wants to merge 3 commits intomainfrom
Conversation
6835e1b to
8438c0f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
==========================================
+ Coverage 38.04% 38.05% +0.01%
==========================================
Files 100 101 +1
Lines 148323 148348 +25
Branches 964 972 +8
==========================================
+ Hits 56424 56449 +25
- Misses 91884 91885 +1
+ Partials 15 14 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 2/3 reviews remaining, refill in 20 minutes. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Charts/Compare/CompareCostsChart.client.vue`:
- Around line 40-41: The chart currently only reacts to changes in costBasis and
diffing, but sortedScenarios (from useSortedScenarios) is the new source of
truth for ordering — add sortedScenarios as a reactive dependency so the chart
rebuilds when order changes: update the watcher/effect that constructs
categories and series (the logic that reads costBasis/diffing to build
categories/series and calls the chart update) to also subscribe to
sortedScenarios (or add a separate watcher on sortedScenarios) and trigger the
same rebuild/update routine; reference sortedScenarios, useSortedScenarios, and
the chart update function so categories/series never become stale when scenario
order updates.
In `@composables/useSortedScenarios.ts`:
- Around line 8-22: The code currently mutates the derived scens array in-place
(scens.sort) and treats missing axis values as indexOf == -1 which places them
before valid options; fix by working on a shallow copy of toValue(scenarios)
(e.g., const sorted = [...toValue(scenarios)]) and sort that copy instead of
scens, and change the comparator around appStore.getScenarioAxisValue to handle
undefined explicitly by mapping missing values to a defined rank (e.g., const
missingRank = sortedOptions.length and use sortedOptions.indexOf(value) >= 0 ?
sortedOptions.indexOf(value) : missingRank) or build a lookup map from
sortedOptions to ranks to avoid indexOf, then return the copied sorted array
(not mutating the original scenarios).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f49cb12f-a664-4c4c-9d2b-8740c2b9b442
📒 Files selected for processing (7)
components/Charts/Compare/CompareCostsChart.client.vuecomponents/Charts/Compare/CompareTimeSeriesLegend.vuecomponents/CostsTable.vuecomposables/useSortedScenarios.tstests/unit/components/Charts/Compare/CompareCostsChart.spec.tstests/unit/components/Charts/Compare/CompareTimeSeriesLegend.spec.tstests/unit/components/CostsTable.spec.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
When parameter metadata specifies an ordering, we should use that to determine the order in which scenarios display, in three places:
Summary by CodeRabbit
Improvements
Tests