Merge pull request #244 from editor-code-assistant/improve-context-handling

Context: skip missing @file refs (reduce prompt noise)

Author: ericdallo

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 - Fix openai-chat tool call + support for Mistral API #233
+- Skip missing/unreadable @file references when building context
 
 ## 0.87.1
 

src/eca/features/context.clj

@@ -27,33 +27,34 @@
   ([path visited]
    (if (contains? visited path)
      []
-     (let [root-content (llm-api/refine-file-context path nil)
-           visited' (conj visited path)
-           at-mentions (extract-at-mentions root-content)
-           parent-dir (str (fs/parent path))
-           resolved-paths (map (fn [mention]
-                                 (cond
-                                   ;; Absolute path
-                                   (string/starts-with? mention "/")
-                                   (str (fs/canonicalize (fs/file mention)))
+     (if-let [root-content (llm-api/refine-file-context path nil)]
+       (let [visited' (conj visited path)
+             at-mentions (extract-at-mentions root-content)
+             parent-dir (str (fs/parent path))
+             resolved-paths (map (fn [mention]
+                                   (cond
+                                     ;; Absolute path
+                                     (string/starts-with? mention "/")
+                                     (str (fs/canonicalize (fs/file mention)))
 
-                                   ;; Relative path (./... or ../...)
-                                   (or (string/starts-with? mention "./")
-                                       (string/starts-with? mention "../"))
-                                   (str (fs/canonicalize (fs/file parent-dir mention)))
+                                     ;; Relative path (./... or ../...)
+                                     (or (string/starts-with? mention "./")
+                                         (string/starts-with? mention "../"))
+                                     (str (fs/canonicalize (fs/file parent-dir mention)))
 
-                                   ;; Simple filename, relative to current file's directory
-                                   :else
-                                   (str (fs/canonicalize (fs/file parent-dir mention)))))
-                               at-mentions)
-           ;; Deduplicate resolved paths
-           unique-paths (distinct resolved-paths)
-           ;; Recursively parse all mentioned files
-           nested-results (mapcat #(parse-agents-file % visited') unique-paths)]
-       (concat [{:type :agents-file
-                 :path path
-                 :content root-content}]
-               nested-results)))))
+                                     ;; Simple filename, relative to current file's directory
+                                     :else
+                                     (str (fs/canonicalize (fs/file parent-dir mention)))))
+                                 at-mentions)
+             ;; Deduplicate resolved paths
+             unique-paths (distinct resolved-paths)
+             ;; Recursively parse all mentioned files
+             nested-results (mapcat #(parse-agents-file % visited') unique-paths)]
+         (concat [{:type :agents-file
+                   :path path
+                   :content root-content}]
+                 nested-results))
+       []))))
 
 (defn agents-file-contexts
   "Search for AGENTS.md file both in workspaceRoot and global config dir.
@@ -75,30 +76,35 @@
          (mapcat #(parse-agents-file (str %))))))
 
 (defn ^:private file->refined-context [path lines-range]
-  (let [ext (string/lower-case (or (fs/extension path) ""))]
-    (if (contains? #{"png" "jpg" "jpeg" "gif" "webp"} ext)
-      {:type :image
-       :media-type (case ext
-                     "jpg" "image/jpeg"
-                     (str "image/" ext))
-       :base64 (.encodeToString (Base64/getEncoder)
-                                (fs/read-all-bytes (fs/file path)))
-       :path path}
-      (assoc-some
-       {:type :file
-        :path path
-        :content (llm-api/refine-file-context path lines-range)}
-       :lines-range lines-range))))
+  (if (fs/readable? path)
+    (let [ext (string/lower-case (or (fs/extension path) ""))]
+      (if (contains? #{"png" "jpg" "jpeg" "gif" "webp"} ext)
+        {:type :image
+         :media-type (case ext
+                       "jpg" "image/jpeg"
+                       (str "image/" ext))
+         :base64 (.encodeToString (Base64/getEncoder)
+                                  (fs/read-all-bytes (fs/file path)))
+         :path path}
+        (when-let [content (llm-api/refine-file-context path lines-range)]
+          (assoc-some
+           {:type :file
+            :path path
+            :content content}
+           :lines-range lines-range))))
+    (logger/warn logger-tag "File not found or unreadable at" path)))
 
 (defn raw-contexts->refined [contexts db]
   (mapcat (fn [{:keys [type path lines-range position uri]}]
             (case (name type)
-              "file" [(file->refined-context path lines-range)]
+              "file" (if-let [ctx (file->refined-context path lines-range)]
+                       [ctx]
+                       [])
               "directory" (->> (fs/glob path "**")
                                (remove fs/directory?)
-                               (map (fn [path]
-                                      (let [filename (str (fs/canonicalize path))]
-                                        (file->refined-context filename nil)))))
+                               (keep (fn [path]
+                                       (let [filename (str (fs/canonicalize path))]
+                                         (file->refined-context filename nil)))))
               "repoMap" [{:type :repoMap}]
               "cursor" [{:type :cursor
                          :path path
@@ -122,16 +128,16 @@
   [prompt db]
   (let [;; Capture @<path> with optional :L<start>-L<end>
         context-pattern #"@([^\s:]+)(?::L(\d+)-L(\d+))?"
-        matches (re-seq context-pattern prompt)]
-    (when (seq matches)
-      (let [raw-contexts (mapv (fn [[_ path s e]]
-                                 (assoc-some {:type "file"
-                                              :path path}
-                                             :lines-range (when (and s e)
-                                                            {:start (Integer/parseInt s)
-                                                             :end (Integer/parseInt e)})))
-                               matches)]
-        (raw-contexts->refined raw-contexts db)))))
+        matches (re-seq context-pattern prompt)
+        raw-contexts (mapv (fn [[_ path s e]]
+                             (assoc-some {:type "file"
+                                          :path path}
+                                         :lines-range (when (and s e)
+                                                        {:start (Integer/parseInt s)
+                                                         :end   (Integer/parseInt e)})))
+                           matches)]
+    (when (seq raw-contexts)
+      (raw-contexts->refined raw-contexts db))))
 
 (defn ^:private all-files-from* [root-filename] (fs/glob root-filename "**"))
 (def ^:private all-files-from (memoize all-files-from*))

src/eca/features/prompt.clj

@@ -68,7 +68,7 @@
                                          content)
                                  (format "<file path=\"%s\">%s</file>\n\n" path content))
                          :agents-file (multi-str
-                                       (format "<agents-file description=\"Instructions following AGENTS.md spec.\" path=\"%s\">" path)
+                                       (format "<agents-file description=\"Primary System Directives & Coding Standards.\" path=\"%s\">" path)
                                        content
                                        "</agents-file>\n\n")
                          :repoMap (format "<repoMap description=\"Workspaces structure in a tree view, spaces represent file hierarchy\" >%s</repoMap>\n\n" @repo-map*)

src/eca/llm_api.clj

@@ -20,15 +20,12 @@
 
 (def ^:private logger-tag "[LLM-API]")
 
-;; TODO ask LLM for the most relevant parts of the path
 (defn refine-file-context [path lines-range]
   (cond
     (not (fs/exists? path))
-    "File not found"
-
+    (logger/warn logger-tag "File not found at" path)
     (not (fs/readable? path))
-    "File not readable"
-
+    (logger/warn logger-tag "Unable to read file at" path)
     :else
     (let [content (slurp path)]
       (if lines-range

test/eca/features/chat_test.clj

@@ -1,5 +1,6 @@
 (ns eca.features.chat-test
   (:require
+   [babashka.fs :as fs]
    [clojure.string :as string]
    [clojure.test :refer [deftest is testing]]
    [eca.features.chat :as f.chat]
@@ -193,9 +194,32 @@
 (deftest contexts-in-prompt-test
   (testing "When prompt contains @file we add a user message"
     (h/reset-components!)
-    (let [{:keys [chat-id]}
+    (with-redefs [fs/readable? (constantly true)
+                  llm-api/refine-file-context (constantly "Mocked file content")]
+      (let [{:keys [chat-id]}
+            (prompt!
+             {:message "Check @/path/to/file please"}
+             {:all-tools-mock (constantly [])
+              :api-mock
+              (fn [{:keys [on-first-response-received
+                           on-message-received]}]
+                (on-first-response-received {:type :text :text "On it..."})
+                (on-message-received {:type :text :text "On it..."})
+                (on-message-received {:type :finish}))})]
+        (is (match?
+             {chat-id {:id chat-id
+                       :messages [{:role "user"
+                                   :content [{:type :text :text "Check @/path/to/file please"}
+                                             {:type :text :text (m/pred #(string/includes? % "<file path"))}]}
+                                  {:role "assistant" :content [{:type :text :text "On it..."}]}]}}
+             (:chats (h/db)))))))
+  (testing "When prompt contains @missing-file we do not add context noise"
+    (h/reset-components!)
+    (let [missing-file "definitely-does-not-exist-eca-test-ctx.md"
+          msg (str "Check @" missing-file " please")
+          {:keys [chat-id]}
           (prompt!
-           {:message "Check @/path/to/file please"}
+           {:message msg}
            {:all-tools-mock (constantly [])
             :api-mock
             (fn [{:keys [on-first-response-received
@@ -205,14 +229,13 @@
               (on-message-received {:type :finish}))})]
       (is (match?
            {chat-id {:id chat-id
-                     :messages [{:role "user" :content [{:type :text :text "Check @/path/to/file please"}
-                                                        {:type :text :text (m/pred #(string/includes? % "<file path"))}]}
+                     :messages [{:role "user" :content [{:type :text :text msg}]}
                                 {:role "assistant" :content [{:type :text :text "On it..."}]}]}}
            (:chats (h/db))))
       (is (match?
            {:chat-content-received
             [{:chat-id chat-id
-              :content {:type :text :text "Check @/path/to/file please\n"}
+              :content {:type :text :text (str msg "\n")}
               :role :user}
              {:chat-id chat-id
               :content {:type :progress :state :running :text "Waiting model"}
@@ -585,9 +608,9 @@
         ;; Rollback to second message (keep first 2 messages, remove last 2)
         (h/reset-messenger!)
         (is (= {} (f.chat/rollback-chat
-                    {:chat-id chat-id
-                     :include ["messages" "tools"]
-                     :content-id second-content-id} (h/db*) (h/messenger))))
+                   {:chat-id chat-id
+                    :include ["messages" "tools"]
+                    :content-id second-content-id} (h/db*) (h/messenger))))
 
         ;; Verify messages after content-id are removed (keeps messages before content-id)
         (is (match?

test/eca/features/context_test.clj

@@ -321,21 +321,39 @@
                 :path b-file
                 :content b-content}])
              (#'f.context/parse-agents-file a-file)))))))
+  (testing "Missing referenced file is ignored"
+    (let [a-file (h/file-path "/fake/AGENTS.md")
+          a-content (multi-str
+                     "- do foo"
+                     "- check @missing.md for missing things")
+          missing-file (h/file-path "/fake/missing.md")]
+      (with-redefs [llm-api/refine-file-context (fn [p _l]
+                                                  (condp = p
+                                                    a-file a-content
+                                                    missing-file nil))]
+        (is (match?
+             (m/in-any-order
+              [{:type :agents-file
+                :path a-file
+                :content a-content}])
+             (#'f.context/parse-agents-file a-file))))))
 
 (deftest contexts-str-from-prompt-test
   (testing "not context mention"
     (is (match?
          nil
          (f.context/contexts-str-from-prompt "check /path/to/file" (h/db)))))
   (testing "Context mention"
-    (with-redefs [llm-api/refine-file-context (constantly "Some content")]
+    (with-redefs [fs/readable? (constantly true)
+                  llm-api/refine-file-context (constantly "Some content")]
       (is (match?
            [{:type :file
              :path "/path/to/file"
              :content "Some content"}]
            (f.context/contexts-str-from-prompt "check @/path/to/file" (h/db))))))
   (testing "Context mention with lines range"
-    (with-redefs [llm-api/refine-file-context (constantly "Some content")]
+    (with-redefs [fs/readable? (constantly true)
+                  llm-api/refine-file-context (constantly "Some content")]
       (is (match?
            [{:type :file
              :path "/path/to/file"