branch: elpa/gptel
commit 11ddace991f63e98212be400570d85d716d2db83
Author: Karthik Chikmagalur <[email protected]>
Commit: Karthik Chikmagalur <[email protected]>

    gptel: Record and restore presets when saving buffers
    
    Change how metadata is stored to files when writing gptel chat
    buffers to disk.  Now it works as follows:
    
    1. If a preset has been applied (globally or in this buffer), save
    a reference to the preset.  This is similar to how we store the
    backend name.
    
    2. For all gptel variable values to be stored (model, backend,
    system message, tools, temperature, max tokens etc), check if
    their current values are as prescribed by the preset.
    
    3. If they are, don't save them.  If they are not, save them as
    before.
    
    When turning on gptel-mode in a chat file, gptel now first applies
    the preset, throwing a warning if this preset is not available.
    Then it applies any remaining stored property values.
    
    The purpose of this change is twofold:
    
    1. Now you can associate a chat buffer with any collection of
    gptel settings via applying a preset, and not just the subset that
    is supported explicitly.
    
    2. Long system prompts often cause the local properties block of
    Markdown chat buffers to exceed 3000 chars, making the chats
    un-resumable.
    See https://github.com/karthink/gptel-agent/issues/60 for an
    example.
    
    Moving the system prompt into a preset avoids this problem.  Of
    course, this has the downside of not making the chat file
    self-contained any more, but this is already the case since we
    store only the backend name.
    
    * gptel.el (gptel--restore-state, gptel--save-state): Save/restore
    presets and metadata according to the above scheme.
    
    * gptel-org.el (gptel-org--restore-state)
    (gptel-org-set-properties): Save/restore presets and metadata
    according to the above scheme.
    
    (gptel-org--send-with-props, gptel-org--entry-properties):
    Supporting changes.
    
    * NEWS (New features and UI changes): Mention changes.
---
 NEWS         |   4 +++
 gptel-org.el | 110 ++++++++++++++++++++++++++++++++++++++---------------------
 gptel.el     |  71 +++++++++++++++++++++++++++-----------
 3 files changed, 128 insertions(+), 57 deletions(-)

diff --git a/NEWS b/NEWS
index bac8cdcb1de..88146ead642 100644
--- a/NEWS
+++ b/NEWS
@@ -45,6 +45,10 @@
   non-nil the LLM can, for instance, read files to fetch more context
   for the rewrite action.
 
+- If a preset has been applied in a gptel chat buffer, saving the
+  buffer to a file causes the preset to be recorded along with the
+  other metadata (model, backend, tools etc).
+
 ** Notable bug fixes
 
 - Function-valued system messages/directives are now evaluated in the
diff --git a/gptel-org.el b/gptel-org.el
index e46f81f2272..4270b59a490 100644
--- a/gptel-org.el
+++ b/gptel-org.el
@@ -499,13 +499,13 @@ parameters.
 
 ARGS are the original function call arguments."
   (if (derived-mode-p 'org-mode)
-      (pcase-let ((`(,gptel--system-message ,gptel-backend ,gptel-model
-                     ,gptel-temperature ,gptel-max-tokens
-                     ,gptel--num-messages-to-send ,gptel-tools)
+      (pcase-let ((`( ,gptel--preset ,gptel--system-message ,gptel-backend
+                      ,gptel-model ,gptel-temperature ,gptel-max-tokens
+                      ,gptel--num-messages-to-send ,gptel-tools)
                    (seq-mapn (lambda (a b) (or a b))
                              (gptel-org--entry-properties)
-                             (list gptel--system-message gptel-backend 
gptel-model
-                                   gptel-temperature gptel-max-tokens
+                             (list gptel--preset gptel--system-message 
gptel-backend
+                                   gptel-model gptel-temperature 
gptel-max-tokens
                                    gptel--num-messages-to-send gptel-tools))))
         (apply send-fun args))
     (apply send-fun args)))
@@ -523,12 +523,13 @@ ARGS are the original function call arguments."
 (defun gptel-org--entry-properties (&optional pt)
   "Find gptel configuration properties stored at PT."
   (pcase-let
-      ((`(,system ,backend ,model ,temperature ,tokens ,num ,tools)
+      ((`(,preset ,system ,backend ,model ,temperature ,tokens ,num ,tools)
          (mapcar
           (lambda (prop) (org-entry-get (or pt (point)) prop 'selective))
-          '("GPTEL_SYSTEM" "GPTEL_BACKEND" "GPTEL_MODEL"
-            "GPTEL_TEMPERATURE" "GPTEL_MAX_TOKENS"
+          '("GPTEL_PRESET" "GPTEL_SYSTEM" "GPTEL_BACKEND"
+            "GPTEL_MODEL" "GPTEL_TEMPERATURE" "GPTEL_MAX_TOKENS"
             "GPTEL_NUM_MESSAGES_TO_SEND" "GPTEL_TOOLS"))))
+    (when preset (setq preset (gptel--intern preset)))
     (when system
       (setq system (string-replace "\\n" "\n" system)))
     (when backend
@@ -548,7 +549,7 @@ ARGS are the original function call arguments."
                    (display-warning
                     '(gptel org tools)
                     (format "Tool %s not found, ignoring" tname)))))
-    (list system backend model temperature tokens num tools)))
+    (list preset system backend model temperature tokens num tools)))
 
 (defun gptel-org--restore-state ()
   "Restore gptel state for Org buffers when turning on `gptel-mode'."
@@ -559,8 +560,17 @@ ARGS are the original function call arguments."
           (progn
             (when-let* ((bounds (org-entry-get (point-min) "GPTEL_BOUNDS")))
               (gptel--restore-props (read bounds)))
-            (pcase-let ((`(,system ,backend ,model ,temperature ,tokens ,num 
,tools)
+            (pcase-let ((`(,preset ,system ,backend ,model ,temperature 
,tokens ,num ,tools)
                          (gptel-org--entry-properties (point-min))))
+              (when preset
+                (if (gptel-get-preset preset)
+                    (progn (gptel--apply-preset
+                            preset (lambda (sym val) (set (make-local-variable 
sym) val)))
+                           (setq gptel--preset preset))
+                  (display-warning
+                   '(gptel presets)
+                   (format "Could not activate gptel preset `%s' in buffer 
\"%s\""
+                           preset (buffer-name)))))
               (when system (setq-local gptel--system-message system))
               (if backend (setq-local gptel-backend backend)
                 (message
@@ -582,36 +592,60 @@ ARGS are the original function call arguments."
 (defun gptel-org-set-properties (pt &optional msg)
   "Store the active gptel configuration under the current heading.
 
-The active gptel configuration includes the current system
-message, language model and provider (backend), and additional
-settings when applicable.
+PT is the cursor position by default.  If MSG is non-nil (default),
+display a message afterwards.
 
-PT is the cursor position by default.  If MSG is
-non-nil (default), display a message afterwards."
+If a gptel preset has been applied in this buffer, a reference to it is
+saved.
+
+Additional metadata is stored only if no preset was applied or if it
+differs from the preset specification.  This is limited to the active
+gptel model and backend names, the system message, active tools, the
+response temperature, max tokens and number of conversation turns to
+send in queries.  (See `gptel--num-messages-to-send' for the last one.)"
   (interactive (list (point) t))
-  (org-entry-put pt "GPTEL_MODEL" (gptel--model-name gptel-model))
-  (org-entry-put pt "GPTEL_BACKEND" (gptel-backend-name gptel-backend))
-  (org-entry-put pt "GPTEL_SYSTEM"      ;TODO: Handle nil case correctly
-                 (and-let* ((msg (car-safe
-                                  (gptel--parse-directive
-                                   gptel--system-message))))
-                   (string-replace "\n" "\\n" msg)))
-  (if gptel-tools
-      (org-entry-put
-       pt "GPTEL_TOOLS" (mapconcat #'gptel-tool-name gptel-tools " "))
-    (org-entry-delete pt "GPTEL_TOOLS"))
-  (if (equal (default-value 'gptel-temperature) gptel-temperature)
-      (org-entry-delete pt "GPTEL_TEMPERATURE")
-    (org-entry-put pt "GPTEL_TEMPERATURE"
-                   (number-to-string gptel-temperature)))
-  (if (natnump gptel--num-messages-to-send)
-      (org-entry-put pt "GPTEL_NUM_MESSAGES_TO_SEND"
-                     (number-to-string gptel--num-messages-to-send))
-    (org-entry-delete pt "GPTEL_NUM_MESSAGES_TO_SEND"))
-  (if gptel-max-tokens
-      (org-entry-put
-       pt "GPTEL_MAX_TOKENS" (number-to-string gptel-max-tokens))
-    (org-entry-delete pt "GPTEL_MAX_TOKENS"))
+  (let ((preset-spec (and gptel--preset (gptel-get-preset gptel--preset))))
+    (if preset-spec
+        (org-entry-put pt "GPTEL_PRESET" (gptel--to-string gptel--preset))
+      (org-entry-delete pt "GPTEL_PRESET"))
+
+    ;; FIXME: nil can mean "no value was explicitly set by the user" as well as
+    ;; "this setting has been set to nil".  We are not yet distinguishing
+    ;; between the two when saving Org properties.  This is particularly
+    ;; relevant for the system message, whose explicit nil value will not be
+    ;; captured when saving Org buffers.
+
+    ;; Model and backend
+    (if (gptel--preset-mismatch-value preset-spec :model gptel-model)
+        (org-entry-put pt "GPTEL_MODEL" (gptel--model-name gptel-model)))
+    (if (gptel--preset-mismatch-value preset-spec :backend gptel-backend)
+        (org-entry-put pt "GPTEL_BACKEND" (gptel-backend-name gptel-backend)))
+    ;; System message
+    (let ((parsed (car-safe (gptel--parse-directive gptel--system-message))))
+      (if (gptel--preset-mismatch-value preset-spec :system parsed)
+          (when parsed
+            (org-entry-put pt "GPTEL_SYSTEM" (string-replace "\n" "\\n" 
parsed)))
+        (org-entry-delete pt "GPTEL_SYSTEM")))
+    ;; Tools
+    (let ((tool-names (mapcar #'gptel-tool-name gptel-tools)))
+      (if (gptel--preset-mismatch-value preset-spec :tools tool-names)
+          (org-entry-put pt "GPTEL_TOOLS" (string-join tool-names " "))
+        (org-entry-delete pt "GPTEL_TOOLS")))
+    ;; Temperature, max tokens and cutoff
+    (if (and (gptel--preset-mismatch-value preset-spec :temperature 
gptel-temperature)
+             (not (equal (default-value 'gptel-temperature) 
gptel-temperature)))
+        (org-entry-put pt "GPTEL_TEMPERATURE" (number-to-string 
gptel-temperature))
+      (org-entry-delete pt "GPTEL_TEMPERATURE"))
+    (if (and (gptel--preset-mismatch-value preset-spec :max-tokens 
gptel-max-tokens)
+             gptel-max-tokens)
+        (org-entry-put pt "GPTEL_MAX_TOKENS" (number-to-string 
gptel-max-tokens))
+      (org-entry-delete pt "GPTEL_MAX_TOKENS"))
+    (if (and (gptel--preset-mismatch-value
+              preset-spec :num-messages-to-send gptel--num-messages-to-send)
+             (natnump gptel--num-messages-to-send))
+        (org-entry-put pt "GPTEL_NUM_MESSAGES_TO_SEND"
+                       (number-to-string gptel--num-messages-to-send))
+      (org-entry-delete pt "GPTEL_NUM_MESSAGES_TO_SEND")))
   (when msg
     (message "Added gptel configuration to current headline.")))
 
diff --git a/gptel.el b/gptel.el
index 1bd4b1b2003..54c6d8d4c7d 100644
--- a/gptel.el
+++ b/gptel.el
@@ -633,6 +633,14 @@ applied before being re-persisted in the new structure."
       (when gptel--bounds
         (gptel--restore-props gptel--bounds)
         (message "gptel chat restored."))
+      (when gptel--preset
+        (if (gptel-get-preset gptel--preset)
+            (gptel--apply-preset
+             gptel--preset (lambda (sym val) (set (make-local-variable sym) 
val)))
+          (display-warning
+           '(gptel presets)
+           (format "Could not activate gptel preset `%s' in buffer \"%s\""
+                   gptel--preset (buffer-name)))))
       (when gptel--backend-name
         (if-let* ((backend (alist-get
                             gptel--backend-name gptel--known-backends
@@ -661,34 +669,59 @@ applied before being re-persisted in the new structure."
 
 This saves chat metadata when writing the buffer to disk.  To
 restore a chat session, turn on `gptel-mode' after opening the
-file."
+file.
+
+If a gptel preset has been applied in this buffer, a reference to it is
+saved.
+
+Additional metadata is stored only if no preset was applied or if it
+differs from the preset specification.  This is limited to the active
+gptel model and backend names, the system message, active tools, the
+response temperature, max tokens and number of conversation turns to
+send in queries.  (See `gptel--num-messages-to-send' for the last one.)"
   (run-hooks 'gptel-save-state-hook)
   (if (derived-mode-p 'org-mode)
       (progn
         (require 'gptel-org)
         (gptel-org--save-state))
-    (let ((print-escape-newlines t))
+    (let ((print-escape-newlines t)
+          (preset-spec (and gptel--preset
+                            (gptel-get-preset gptel--preset))))
       (save-excursion
         (save-restriction
-          (add-file-local-variable 'gptel-model gptel-model)
-          (add-file-local-variable 'gptel--backend-name
-                                   (gptel-backend-name gptel-backend))
-          (unless (equal (default-value 'gptel--system-message)
-                           gptel--system-message)
-            (add-file-local-variable
-             'gptel--system-message     ;TODO: Handle nil case correctly
-             (car-safe (gptel--parse-directive gptel--system-message))))
-          (if gptel-tools
-              (add-file-local-variable
-               'gptel--tool-names (mapcar #'gptel-tool-name gptel-tools))
-            (delete-file-local-variable 'gptel--tool-names))
-          (if (equal (default-value 'gptel-temperature) gptel-temperature)
-              (delete-file-local-variable 'gptel-temperature)
-            (add-file-local-variable 'gptel-temperature gptel-temperature))
-          (if gptel-max-tokens
+
+          (if preset-spec
+              (add-file-local-variable 'gptel--preset gptel--preset)
+            (delete-file-local-variable 'gptel--preset))
+
+          ;; Model and backend
+          (if (gptel--preset-mismatch-value preset-spec :model gptel-model)
+              (add-file-local-variable 'gptel-model gptel-model))
+          (if (gptel--preset-mismatch-value preset-spec :backend gptel-backend)
+              (add-file-local-variable 'gptel--backend-name
+                                       (gptel-backend-name gptel-backend)))
+          ;; System message
+          (let ((parsed (car-safe (gptel--parse-directive 
gptel--system-message))))
+            (if (gptel--preset-mismatch-value preset-spec :system parsed)
+                (add-file-local-variable 'gptel--system-message parsed)
+              (delete-file-local-variable 'gptel--system-message)))
+          ;; Tools
+          (let ((tool-names (mapcar #'gptel-tool-name gptel-tools)))
+            (if (gptel--preset-mismatch-value preset-spec :tools tool-names)
+                (add-file-local-variable 'gptel--tool-names tool-names)
+              (delete-file-local-variable 'gptel--tool-names)))
+          ;; Temperature, max tokens and cutoff
+          (if (and (gptel--preset-mismatch-value preset-spec :temperature 
gptel-temperature)
+                   (not (equal (default-value 'gptel-temperature) 
gptel-temperature)))
+              (add-file-local-variable 'gptel-temperature gptel-temperature)
+            (delete-file-local-variable 'gptel-temperature))
+          (if (and (gptel--preset-mismatch-value preset-spec :max-tokens 
gptel-max-tokens)
+                   gptel-max-tokens)
               (add-file-local-variable 'gptel-max-tokens gptel-max-tokens)
             (delete-file-local-variable 'gptel-max-tokens))
-          (if (natnump gptel--num-messages-to-send)
+          (if (and (gptel--preset-mismatch-value
+                    preset-spec :num-messages-to-send 
gptel--num-messages-to-send)
+                   (natnump gptel--num-messages-to-send))
               (add-file-local-variable 'gptel--num-messages-to-send
                                        gptel--num-messages-to-send)
             (delete-file-local-variable 'gptel--num-messages-to-send))

Reply via email to