16 июля 2024 г., в 19:54, Philip Kaludercic <philipk@posteo.net> написал(а):

Despite
reading through the entire source code, I have no idea what you are
trying to do with this package. It would be nice to have some more
context in the Commentary section and elaborate a number of user-facing

docstrings, unless this is intended to be expert software, for people
familiar with whatever the field is.

I need to improve it. With README.org it should be cleaner, but It should be understandable without it.

Why should we remove elisa customization group? I think there are many customization options to move it to group.

@@ -48,68 +47,61 @@
   (make-llm-ollama
    :embedding-model "nomic-embed-text"))
  "Embeddings provider to generate embeddings."
- :group 'elisa
- :type '(sexp :validate 'cl-struct-p))
+ :type '(sexp :validate 'cl-struct-p)) ;a more specific predicate here?

It should be `llm` provider. Unfortunately there are no more specific predicate for that.

(defcustom elisa-db-directory (file-truename
      (file-name-concat
user-emacs-directory "elisa"))
  "Directory for elisa database."
- :group 'elisa
- :type 'directory)
+ :type 'directory)                     ;is it necessary that it exists?

If you mean option - then yes. If you mean directory, I can create it in `elisa' package.

+;; a number of these functions seem like something that should be added to the core of Emacs or at least a common ELPA package...
(defun elisa-dot-product (v1 v2)
"Calculate the dot produce of vectors V1 and V2."

Good idea. How can we implement it?

-       (format "%s -f html --to plain"
-       (executable-find elisa-pandoc-executable))
+       (format "%s --from html --to plain" elisa-pandoc-executable)
       buffer-name t)
      buffer-name)))

We should keep executable-find call here. It was feature request from users. It can be useful with per-directory PATH, like with direnv.

(defun elisa-fts-query (prompt)
  "Return fts match query for PROMPT."
- (thread-last
+ (thread-last                          ;i belive you can do all of this with a single regular expression...
    prompt
    (string-trim)
    (downcase)

Maybe. But I prefer to keep it readable.

@@ -1071,7 +1044,7 @@ WHERE d.rowid in %s;"
  (when-let ((kind (cl-first row))
     (path (cl-second row))
     (text (cl-third row)))
-     (pcase kind
+     (pcase kind        ;is this a `pcase-exhaustive'?
      ("web"
(ellama-context-add-webpage-quote-noninteractive path path text))
      ("file"

I think it should be extendable, but for now it is not.

Best regards,

Sergey Kostyaev