feat: Add generate deeplink tool for dashboards, panels, and explore queries #218
Conversation
There was a problem hiding this comment.
Nice, @iamrajiv, this looks awesome.
I'd love to see an end-to-end test for this (in the tests directory), it'd be a great way to demo how it looks. Do you think you could have a go at that?
Could you also update the README with the new tool?
There was a problem hiding this comment.
Thanks for adding docs & tests, I'm trying to run the tests locally but they're not all passing, perhaps some of the requirements are a bit strict. Honestly they're mostly smoke tests since it's impossible to control the underlying models so you can simplify them to just include the first one or two, that should be less flaky.
|
thanks for the review @sd2k. i’ve removed the unnecessary file i had created those to test my stuff and yes we can use make run i just noticed the makefile now also i’ve updated the readme to include this feature and we have e2e tests too which are working fine do you think we should have a demo video as well like how to connect usage all mcp features and all that not in this pr but maybe in a separate issue and improvement? |
There was a problem hiding this comment.
Thanks again @iamrajiv!
I'm still running into failures with the e2e tests (i.e. when I run (cd tests && uv run pytest navigation_test.py)). I think we can make the LLM-as-a-judge requirements a bit less strict (and perhaps we can get rid of the 'workflow' test case). The suggestion I made inline about removing the more vague UID parameter might help, not sure though.
do you think we should have a demo video as well like how to connect usage all mcp features and all that not in this pr but maybe in a separate issue and improvement?
I think some demo videos showing off the various tools would be useful yeah. I'll create a separate issue for that.
|
also @sd2k the e2e tests pass locally but they need paid api keys and sometimes run into rate limits while testing since we already use string-based checks that cover the same logic should we make the llm checks optional for contributors without api credits? |
|
Hey @iamrajiv, I'm happy to run the e2e tests and sort them out, it looks like just the |
Fixes: #166