fix(mcp): Skip misleading trend analysis for categorical ASCII charts#39761
Conversation
Code Review Agent Run #0b9429Actionable Suggestions - 0Additional SuggestionsFiltered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules. Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #39761 +/- ##
==========================================
- Coverage 64.16% 63.86% -0.30%
==========================================
Files 2591 2585 -6
Lines 138302 136940 -1362
Branches 32084 31522 -562
==========================================
- Hits 88736 87457 -1279
+ Misses 48036 47953 -83
Partials 1530 1530
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…andrusoare/fix/mcp_skip_categorical_charts
|
Bito Automatic Review Skipped – PR Already Merged |
SUMMARY
Problem
When an LLM asks for an ASCII chart preview of categorical data (e.g., countries sorted alphabetically), the trend analysis compares first vs last data points and reports statements like "Strong downward trend." This is meaningless — the ordering is alphabetical, not temporal — and misleads LLMs into drawing incorrect conclusions about the data.
Root Cause
_analyze_trend()unconditionally comparesvalues[0]vsvalues[-1]to determine directional trend, assuming the data is temporally ordered. It has no awareness of whether the x-axis represents time or categories._extract_time_series_data()already distinguishes date columns from categorical ones (via keyword matching on column names), but this signal was discarded — never propagated to the trend analysis.Fix
_extract_time_series_datanow returns anis_temporalflag (Truewhen the label column name contains date/time keywords)_analyze_trendaccepts this flag and replaces directional trend statements with a categorical caveat whenis_temporal=FalseBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION