fix(ai-chat): send the page's own params where the chat reads them - #2309
Open
Fiona2016 wants to merge 2 commits into
Open
fix(ai-chat): send the page's own params where the chat reads them#2309Fiona2016 wants to merge 2 commits into
Fiona2016 wants to merge 2 commits into
Conversation
In the commercial build the chat is srm-fe's, and a page hands it context through setParamsAiAction. That side strips the fields it uses itself and sends everything left in `custom` as page_from.param — a flat bag, beside workspace_id and the firemap keys. We were spreading the wrappers instead of their contents. `queryPageFrom` went in whole, so the page's params landed a level deeper at param.param where nothing looks; `queryAction` went in whole too, so its `param` overwrote the page's and its `key` arrived as a stray field rather than under `action`. The metric explorer therefore told the model nothing about which data source the user had selected, and the model answered from what metric names usually look like. Spread `queryPageFrom.param` and nest the action under `action`. `url` is deliberately not carried: the chat reads the address bar, which names the same page and stays right across a navigation. Also send `panelKey`. Metric.tsx has always compared it to decide whether removing a panel should close the chat, but nothing ever wrote it, so that comparison could never hold.
The commercial build replaces App.tsx and never mounts the provider Metric.tsx reads, so the close-on-remove it enables is open-source only.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe AI chat handler now builds the expected custom payload from page parameters and action data. Prometheus passes ChangesAI chat context
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change corrects AI chat context payloads so metric explorer page parameters and panel identity are available to chat. No current merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
In the commercial build the chat is srm-fe's, and a page hands it context through
setParamsAiAction. That side (srm-feChatContent/index.tsx,formatPageInfo) strips the fields it uses itself and sends everything left incustomaspage_from.param— a flat bag, besideworkspace_id,business_idand the firemap keys.We were spreading the wrappers instead of their contents:
...queryPageFromput{url, param}in whole, so the page's own params landed a level deeper atpage_from.param.param, where nothing looks....queryActiondid the same, so itsparamoverwrote the page's, and itskeyarrived as a stray field rather than underaction— which is the field srm-fe actually destructures and re-adds.The visible consequence: the metric explorer told the model nothing about which data source the user had selected, so it answered from what metric names usually look like. Measured on 106, asking for a host CPU query returned four candidate expressions and this handed back to the user:
That data source has no
hostnamelabel — its host identity label isident.What changes
Spread
queryPageFrom.param, and nest the action underaction.urlis deliberately not carried: the chat sets it from the address bar, which names the same page and stays right across a navigation. Checked everyAiButton/CustomAiButtonWrapcall site — all buildqueryPageFromthroughbuildPageFrom({param}), so no caller relied on any top-level field surviving the spread.Also sends
panelKey.Metric.tsxhas always compared it to decide whether removing a panel should close the chat, but nothing ever wrote it, so that comparison could never hold. It reaches that comparison in the open-source build only — the commercial one replacesApp.tsxand never mounts the providerMetric.tsxreads from; the comment says so.Testing
FlashAiButton.test.tsnow pins both halves of the shape. It asserts against source text rather than rendering, becauseIS_ENTreadsimport.meta.envand cannot be evaluated under this project's jest transform — so a rendered test would only ever exercise the CE branch. The file's header says this.jest src/components/AiChatNG src/pages/explorer— 32 passing.tsc --noEmitmatchesorigin/mainexactly (52 pre-existing errors, none in these files).Related
The backend half is
flashcatcloud/fc-model-server#210, which reads these params and delivers the verified query as a card. Neither is useful without the other, but they land independently.Supersedes
#2305, which took a different approach (registering an ai-kit page action) that cannot work in the commercial build — the action would register in n9e/fe's runtime while srm-fe's chat reads its own.Summary by CodeRabbit
Bug Fixes
Tests