From 3eb1a0ccaa37410e6fe0059a9cb10145efa0d615 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 6 Mar 2026 21:15:31 -0600 Subject: refactor(gptel): lazy-load gptel-magit, rebind rewrite/context keys - Replace use-package gptel-magit hook with autoloads via with-eval-after-load 'magit (loads gptel only on key press) - Move org header defuns above use-package to fix load order - Set gptel-include-reasoning to "*AI-Reasoning*" buffer - Rebind rewrite to C-; a r, clear context to C-; a c - Add test-ai-config-gptel-magit-lazy-loading.el (8 tests) - Mark all ai-config cleanup items DONE in todo.org Co-Authored-By: Claude Opus 4.6 --- modules/ai-config.el | 76 ++++++++------ tests/test-ai-config-gptel-magit-lazy-loading.el | 127 +++++++++++++++++++++++ tests/testutil-ai-config.el | 7 +- todo.org | 6 +- 4 files changed, 181 insertions(+), 35 deletions(-) create mode 100644 tests/test-ai-config-gptel-magit-lazy-loading.el diff --git a/modules/ai-config.el b/modules/ai-config.el index 0df2a050..d8e73cf9 100644 --- a/modules/ai-config.el +++ b/modules/ai-config.el @@ -255,6 +255,32 @@ Works for any buffer, whether it's visiting a file or not." (gptel-add '(4)) (message "Added buffer '%s' to GPTel context" (buffer-name))) +;;; -------------------------- Org Header Construction -------------------------- + +(defun cj/gptel--fresh-org-prefix () + "Generate a fresh org-mode header with current timestamp for user messages." + (concat "* " user-login-name " " (format-time-string "[%Y-%m-%d %H:%M:%S]") "\n")) + +(defun cj/gptel--refresh-org-prefix (&rest _) + "Update the org-mode prefix with fresh timestamp before sending message." + (setf (alist-get 'org-mode gptel-prompt-prefix-alist) + (cj/gptel--fresh-org-prefix))) + +(defun cj/gptel-backend-and-model () + "Return backend, model, and timestamp as a single string." + (let* ((backend (pcase (bound-and-true-p gptel-backend) + ((and v (pred vectorp)) (aref v 1)) + (_ "AI"))) + (model (format "%s" (or (bound-and-true-p gptel-model) ""))) + (ts (format-time-string "[%Y-%m-%d %H:%M:%S]"))) + (format "%s: %s %s" backend model ts))) + +(defun cj/gptel-insert-model-heading (response-begin-pos _response-end-pos) + "Insert an Org heading for the AI reply at RESPONSE-BEGIN-POS." + (save-excursion + (goto-char response-begin-pos) + (insert (format "* %s\n" (cj/gptel-backend-and-model))))) + ;;; ---------------------------- GPTel Configuration ---------------------------- (use-package gptel @@ -286,32 +312,6 @@ Works for any buffer, whether it's visiting a file or not." (advice-add 'gptel-send :before #'cj/gptel--refresh-org-prefix) (add-hook 'gptel-post-response-functions #'cj/gptel-insert-model-heading)) -;;; -------------------------- Org Header Construction -------------------------- - -(defun cj/gptel--fresh-org-prefix () - "Generate a fresh org-mode header with current timestamp for user messages." - (concat "* " user-login-name " " (format-time-string "[%Y-%m-%d %H:%M:%S]") "\n")) - -(defun cj/gptel--refresh-org-prefix (&rest _) - "Update the org-mode prefix with fresh timestamp before sending message." - (setf (alist-get 'org-mode gptel-prompt-prefix-alist) - (cj/gptel--fresh-org-prefix))) - -(defun cj/gptel-backend-and-model () - "Return backend, model, and timestamp as a single string." - (let* ((backend (pcase (bound-and-true-p gptel-backend) - ((and v (pred vectorp)) (aref v 1)) - (_ "AI"))) - (model (format "%s" (or (bound-and-true-p gptel-model) ""))) - (ts (format-time-string "[%Y-%m-%d %H:%M:%S]"))) - (format "%s: %s %s" backend model ts))) - -(defun cj/gptel-insert-model-heading (response-begin-pos _response-end-pos) - "Insert an Org heading for the AI reply at RESPONSE-BEGIN-POS." - (save-excursion - (goto-char response-begin-pos) - (insert (format "* %s\n" (cj/gptel-backend-and-model))))) - ;;; ---------------------------- Toggle GPTel Window ---------------------------- (defun cj/toggle-gptel () @@ -359,9 +359,27 @@ Works for any buffer, whether it's visiting a file or not." ;;; -------------------------------- GPTel-Magit -------------------------------- -(use-package gptel-magit - :defer t - :hook (magit-mode . gptel-magit-install)) +;; Lazy gptel-magit integration (replaces use-package gptel-magit block). +;; +;; The original `(magit-mode . gptel-magit-install)' hook ran on every magit +;; buffer, repeatedly re-installing keybindings and loading gptel eagerly. +;; +;; Instead, we register autoloads and wire up keybindings once when magit +;; loads. gptel-magit (and gptel) are only loaded when you actually press +;; one of these keys: +;; M-g — generate commit message (in commit message buffer) +;; g — generate commit (in magit-commit transient) +;; x — explain diff (in magit-diff transient) + +(with-eval-after-load 'magit + (autoload 'gptel-magit-generate-message "gptel-magit" nil t) + (autoload 'gptel-magit-commit-generate "gptel-magit" nil t) + (autoload 'gptel-magit-diff-explain "gptel-magit" nil t) + (define-key git-commit-mode-map (kbd "M-g") #'gptel-magit-generate-message) + (transient-append-suffix 'magit-commit #'magit-commit-create + '("g" "Generate commit" gptel-magit-commit-generate)) + (transient-append-suffix 'magit-diff #'magit-stash-show + '("x" "Explain" gptel-magit-diff-explain))) ;; ------------------------------ GPTel Directives ----------------------------- diff --git a/tests/test-ai-config-gptel-magit-lazy-loading.el b/tests/test-ai-config-gptel-magit-lazy-loading.el new file mode 100644 index 00000000..05e3634d --- /dev/null +++ b/tests/test-ai-config-gptel-magit-lazy-loading.el @@ -0,0 +1,127 @@ +;;; test-ai-config-gptel-magit-lazy-loading.el --- Tests for gptel-magit lazy loading -*- lexical-binding: t; -*- + +;;; Commentary: +;; Tests for the lazy gptel-magit integration in ai-config.el. +;; +;; ai-config.el uses `with-eval-after-load 'magit' to register autoloads +;; and keybindings for gptel-magit functions. This avoids loading gptel +;; on every magit buffer (the old hook approach) and defers it until the +;; user actually presses a gptel-magit key. +;; +;; Testing approach: +;; - Load ai-config with gptel stubs (via testutil-ai-config) but WITHOUT +;; providing magit, so the eval-after-load block does not fire yet. +;; - Provide a minimal magit stub (just the keymap and transient helpers), +;; which triggers the eval-after-load and wires up the bindings. +;; - Verify that gptel-magit functions are registered as autoloads (not +;; fully loaded), confirming deferred loading works. +;; - Verify the M-g keymap binding in git-commit-mode-map. +;; +;; What is NOT tested here (requires real magit transients): +;; - transient-append-suffix actually adds "g" to magit-commit +;; - transient-append-suffix actually adds "x" to magit-diff +;; These are best verified manually in a running Emacs. + +;;; Code: + +(require 'ert) + +(add-to-list 'load-path (expand-file-name "tests" user-emacs-directory)) +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) + +;; Load gptel stubs — this does NOT provide 'magit, so the +;; with-eval-after-load 'magit block in ai-config stays dormant. +(require 'testutil-ai-config) + +;; Stub magit's keymap and transient infrastructure. These must exist +;; before we trigger the eval-after-load, since ai-config uses them to +;; set up keybindings and transient entries. +(defvar git-commit-mode-map (make-sparse-keymap) + "Stub keymap standing in for magit's git-commit-mode-map.") + +;; Stub transient-append-suffix to be a no-op. We can't test transient +;; integration without real magit transient definitions, so we just +;; verify it doesn't error. +(defvar test-gptel-magit--transient-calls nil + "Records calls to the stubbed transient-append-suffix for verification.") + +(defun transient-append-suffix (prefix loc suffix) + "Stub: record the call for test inspection instead of modifying transients. +PREFIX is the transient being modified, LOC is the reference suffix, +SUFFIX is the entry being appended." + (push (list prefix loc suffix) test-gptel-magit--transient-calls)) + +;; Now load ai-config. The with-eval-after-load 'magit block is +;; registered but NOT executed yet. +(require 'ai-config) + +;; ----------------------------- Setup / Teardown ------------------------------ + +(defun test-gptel-magit-setup () + "Trigger the magit eval-after-load by providing the feature. +This simulates what happens when a user first opens magit." + (setq test-gptel-magit--transient-calls nil) + (provide 'magit)) + +;; NOTE: We cannot un-provide 'magit between tests, and eval-after-load +;; callbacks only fire once, so all tests share the same post-trigger +;; state. The setup runs once before the first test. + +(test-gptel-magit-setup) + +;;; Normal Cases — Autoloads + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-generate-message-is-autoload () + "After magit loads, gptel-magit-generate-message should be an autoload. +An autoload means the function is registered but gptel-magit.el has not +been loaded yet — it will only load when the function is first called." + (should (fboundp 'gptel-magit-generate-message)) + (should (autoloadp (symbol-function 'gptel-magit-generate-message)))) + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-commit-generate-is-autoload () + "After magit loads, gptel-magit-commit-generate should be an autoload." + (should (fboundp 'gptel-magit-commit-generate)) + (should (autoloadp (symbol-function 'gptel-magit-commit-generate)))) + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-diff-explain-is-autoload () + "After magit loads, gptel-magit-diff-explain should be an autoload." + (should (fboundp 'gptel-magit-diff-explain)) + (should (autoloadp (symbol-function 'gptel-magit-diff-explain)))) + +;;; Normal Cases — Keymap Binding + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-keymap-binding () + "M-g in git-commit-mode-map should be bound to gptel-magit-generate-message. +This binding allows generating a commit message from the commit buffer." + (let ((bound (lookup-key git-commit-mode-map (kbd "M-g")))) + (should (eq bound 'gptel-magit-generate-message)))) + +;;; Normal Cases — Transient Registration + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-transient-commit-registered () + "transient-append-suffix should have been called for magit-commit. +Verifies that the 'g' / Generate commit entry was registered." + (should (cl-find 'magit-commit test-gptel-magit--transient-calls + :key #'car))) + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-transient-diff-registered () + "transient-append-suffix should have been called for magit-diff. +Verifies that the 'x' / Explain entry was registered." + (should (cl-find 'magit-diff test-gptel-magit--transient-calls + :key #'car))) + +;;; Boundary Cases + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-boundary-gptel-magit-not-loaded () + "The gptel-magit feature should NOT be loaded after magit triggers. +Only autoloads are registered — the actual package stays unloaded until +a gptel-magit function is called." + (should-not (featurep 'gptel-magit))) + +(ert-deftest test-ai-config-gptel-magit-lazy-loading-boundary-exactly-two-transient-calls () + "Exactly two transient-append-suffix calls should have been made. +One for magit-commit (generate) and one for magit-diff (explain)." + (should (= 2 (length test-gptel-magit--transient-calls)))) + +(provide 'test-ai-config-gptel-magit-lazy-loading) +;;; test-ai-config-gptel-magit-lazy-loading.el ends here diff --git a/tests/testutil-ai-config.el b/tests/testutil-ai-config.el index 4839efd5..e8953389 100644 --- a/tests/testutil-ai-config.el +++ b/tests/testutil-ai-config.el @@ -63,9 +63,10 @@ (defun gptel-prompts-add-update-watchers (&rest _) "Stub." nil) (provide 'gptel-prompts) -;; Stub gptel-magit -(defun gptel-magit-install (&rest _) "Stub." nil) -(provide 'gptel-magit) +;; NOTE: gptel-magit is NOT stubbed here. ai-config.el now uses +;; with-eval-after-load 'magit instead of use-package gptel-magit, +;; so the magit integration only activates when magit is provided. +;; See test-ai-config-gptel-magit-lazy-loading.el for magit stub tests. ;; Stub ai-conversations (provide 'ai-conversations) diff --git a/todo.org b/todo.org index e71def90..e4722093 100644 --- a/todo.org +++ b/todo.org @@ -173,9 +173,9 @@ Changed to ~"*AI-Reasoning*"~ — reasoning redirected to a separate buffer to keep conversations clean while remaining reviewable. Not re-sent as context (saves API tokens). Can be toggled per-session via gptel menu (~C-; a M~). -**** ~gptel-magit~ loads on every magit open -Hook ~(magit-mode . gptel-magit-install)~ runs every magit session even when -AI commit messages aren't needed. Minor overhead. +**** DONE ~gptel-magit~ loads on every magit open +Replaced use-package hook with lazy autoloads via ~with-eval-after-load 'magit~. +Keybindings wired up once when magit loads; gptel-magit only loads on key press. **** DONE Rewrite bound to ~&~ — unusual mnemonic Rewrite moved to ~C-; a r~, clear context moved to ~C-; a c~. -- cgit v1.2.3