khoj-ai / khoj

Your AI second brain. Self-hostable. Get answers from the web or your docs. Build custom agents, schedule automations, do deep research. Turn any online or local LLM into your personal, autonomous AI (e.g gpt, claude, gemini, llama, qwen, mistral).
https://khoj.dev
GNU Affero General Public License v3.0
14.22k stars 705 forks source link

fix: Fix local server usage #559

Closed dtkav closed 11 months ago

dtkav commented 11 months ago

fixes #558

dtkav commented 11 months ago

Hey @debanjum , thanks for the review. While this fix makes it possible to use local servers again (I am using it in my fork via BRAT ), I don't think it is ideal.

AFAICT this is still the only network check that updates the connection status setting, and command registration only runs when you load the plugin. This means that you need to configure the plugin via the settings page and then restart obsidian for it to take effect. This is an unexpected flow compared to other obsidian plugins I have used.

Here are some ideas to improve the UX:

  1. Automatically test the connection on-edit of the url/api key fields
  2. update settings.connectedToBackend to true on any successful network request to the API.
  3. Test the connection in the command handler, perhaps with some debounce rather than relying on a settings.connectedToBackend value which could be stale.
  4. Allow the search and chat commands to be registered regardless of the flag value and then show the connectivity error message in the search/chat modal.

IMHO it makes sense to: Immediate term -- merge this so that there is a workaround for the regression. Short term -- test the network connection and set settings.connectedToBackend on edit to the url/api key fields (or with an explicit "test" button). Longer term --

  1. Move the connectivity state management into a wrapper around the network layer
  2. Surface the connectivity state in the various modals rather than to inexplicably block command registration/invocation which makes it seem like the installation is broken.

I have only skimmed the code base, so please take the above with a grain of salt.

Thanks for your work on Khoj.

debanjum commented 11 months ago

Hey @dtkav, thanks for the well thought-out suggestions 👌🏾. Agree with the immediate, short and long term improvement suggestions for managing backend connectivity state. I'll merge this fix of yours in for now. And look into the short term solution in a bit