Skip to content

Commit eb175b8

Browse files
ericdalloericdallo
committed
Improve eca_read_file
1 parent 179a70e commit eb175b8

File tree

4 files changed

+94
-66
lines changed

4 files changed

+94
-66
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
- Improve `eca_read_file` tool to have better and more assertive descriptions/parameters.
6+
57
## 0.10.0
68

79
- Increase anthropic models maxTokens to 8196

‎src/eca/features/tools/filesystem.clj‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,20 @@
3030
""
3131
(fs/list-dir @path))))))
3232

33+
(def ^:private read-file-max-lines 2000)
34+
3335
(defn ^:private read-file [arguments {:keys [db]}]
3436
(or (tools.util/invalid-arguments arguments (concat (path-validations db)
3537
[["path" fs/readable? "File $path is not readable"]]))
36-
(let [head (get arguments "head")
37-
tail (get arguments "tail")
38+
(let [line-offset (get arguments "line_offset")
39+
limit (or (get arguments "limit") read-file-max-lines)
3840
content (cond-> (slurp (fs/file (fs/canonicalize (get arguments "path"))))
39-
head (->> (string/split-lines)
40-
(take head)
41-
(string/join "\n"))
42-
tail (->> (string/split-lines)
43-
(take-last tail)
44-
(string/join "\n")))]
41+
line-offset (->> (string/split-lines)
42+
(drop line-offset)
43+
(string/join "\n"))
44+
limit (->> (string/split-lines)
45+
(take limit)
46+
(string/join "\n")))]
4547
(tools.util/single-text-content content))))
4648

4749
(defn ^:private write-file [arguments {:keys [db]}]
@@ -205,17 +207,16 @@
205207
{:description (str "Read the complete contents of a file from the file system. "
206208
"Handles various text encodings and provides detailed error messages "
207209
"if the file cannot be read. Use this tool when you need to examine "
208-
"the contents of a single file. Use the 'head' parameter to read only "
209-
"the first N lines of a file, or the 'tail' parameter to read only "
210-
"the last N lines of a file."
210+
"the contents of a single file. Optionally use the 'line_offset' and/or 'limit' "
211+
"parameters to read specific contents of the file when you know the lines."
211212
"**Only works within the directories: $workspaceRoots.**")
212213
:parameters {:type "object"
213214
:properties {"path" {:type "string"
214215
:description "The absolute path to the file to read."}
215-
"head" {:type "integer"
216-
:description "If provided, returns only the first N lines of the file"}
217-
"tail" {:type "integer"
218-
:description "If provided, returns only the last N lines of the file"}}
216+
"line_offset" {:type "integer"
217+
:description "Line to start reading from (default: 0)"}
218+
"limit" {:type "integer"
219+
:description (str "Maximum lines to read (default: " read-file-max-lines ")")}}
219220
:required ["path"]}
220221
:handler #'read-file}
221222
"eca_write_file"

‎test/eca/features/tools/filesystem_test.clj‎

Lines changed: 61 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -76,31 +76,41 @@
7676
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
7777
{"path" (h/file-path "/foo/qux")}
7878
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
79-
(testing "heading a file"
79+
(testing "with line_offset"
8080
(is (match?
8181
{:contents [{:type :text
8282
:error false
83-
:content "fooo\nbar"}]}
84-
(with-redefs [slurp (constantly "fooo\nbar\nbaz")
83+
:content "line3\nline4\nline5"}]}
84+
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
8585
fs/exists? (constantly true)
8686
fs/readable? (constantly true)
8787
f.tools.filesystem/allowed-path? (constantly true)]
8888
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
89-
{"path" (h/file-path "/foo/qux")
90-
"head" 2}
89+
{"path" (h/file-path "/foo/qux") "line_offset" 2}
9190
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
92-
(testing "tailling a file"
91+
(testing "with limit"
9392
(is (match?
9493
{:contents [{:type :text
9594
:error false
96-
:content "bar\nbaz"}]}
97-
(with-redefs [slurp (constantly "fooo\nbar\nbaz")
95+
:content "line1\nline2"}]}
96+
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
9897
fs/exists? (constantly true)
9998
fs/readable? (constantly true)
10099
f.tools.filesystem/allowed-path? (constantly true)]
101100
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
102-
{"path" (h/file-path "/foo/qux")
103-
"tail" 2}
101+
{"path" (h/file-path "/foo/qux") "limit" 2}
102+
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
103+
(testing "with line_offset and limit"
104+
(is (match?
105+
{:contents [{:type :text
106+
:error false
107+
:content "line3\nline4"}]}
108+
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
109+
fs/exists? (constantly true)
110+
fs/readable? (constantly true)
111+
f.tools.filesystem/allowed-path? (constantly true)]
112+
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
113+
{"path" (h/file-path "/foo/qux") "line_offset" 2 "limit" 2}
104114
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}}))))))
105115

106116
(deftest write-file-test
@@ -119,53 +129,53 @@
119129
(deftest search-files-test
120130
(testing "invalid pattern"
121131
(is (match?
122-
{:contents [{:type :text
123-
:error true
124-
:content "Invalid glob pattern ' '"}]}
125-
(with-redefs [fs/exists? (constantly true)]
126-
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
127-
{"path" (h/file-path "/project/foo")
128-
"pattern" " "}
129-
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
132+
{:contents [{:type :text
133+
:error true
134+
:content "Invalid glob pattern ' '"}]}
135+
(with-redefs [fs/exists? (constantly true)]
136+
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
137+
{"path" (h/file-path "/project/foo")
138+
"pattern" " "}
139+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
130140
(testing "no matches"
131141
(is (match?
132-
{:contents [{:type :text
133-
:error false
134-
:content "No matches found"}]}
135-
(with-redefs [fs/exists? (constantly true)
136-
fs/glob (constantly [])]
137-
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
138-
{"path" (h/file-path "/project/foo")
139-
"pattern" "foo"}
140-
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
142+
{:contents [{:type :text
143+
:error false
144+
:content "No matches found"}]}
145+
(with-redefs [fs/exists? (constantly true)
146+
fs/glob (constantly [])]
147+
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
148+
{"path" (h/file-path "/project/foo")
149+
"pattern" "foo"}
150+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
141151
(testing "matches with wildcard"
142152
(is (match?
143-
{:contents [{:type :text
144-
:error false
145-
:content (str (h/file-path "/project/foo/bar/baz.txt") "\n"
146-
(h/file-path "/project/foo/qux.txt") "\n"
147-
(h/file-path "/project/foo/qux.clj"))}]}
148-
(with-redefs [fs/exists? (constantly true)
149-
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
150-
(fs/path (h/file-path "/project/foo/qux.txt"))
151-
(fs/path (h/file-path "/project/foo/qux.clj"))])]
152-
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
153-
{"path" (h/file-path "/project/foo")
154-
"pattern" "**"}
155-
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
153+
{:contents [{:type :text
154+
:error false
155+
:content (str (h/file-path "/project/foo/bar/baz.txt") "\n"
156+
(h/file-path "/project/foo/qux.txt") "\n"
157+
(h/file-path "/project/foo/qux.clj"))}]}
158+
(with-redefs [fs/exists? (constantly true)
159+
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
160+
(fs/path (h/file-path "/project/foo/qux.txt"))
161+
(fs/path (h/file-path "/project/foo/qux.clj"))])]
162+
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
163+
{"path" (h/file-path "/project/foo")
164+
"pattern" "**"}
165+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
156166
(testing "matches without wildcard"
157167
(is (match?
158-
{:contents [{:type :text
159-
:error false
160-
:content (str (h/file-path "/project/foo/bar/baz.txt") "\n"
161-
(h/file-path "/project/foo/qux.txt"))}]}
162-
(with-redefs [fs/exists? (constantly true)
163-
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
164-
(fs/path (h/file-path "/project/foo/qux.txt"))])]
165-
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
166-
{"path" (h/file-path "/project/foo")
167-
"pattern" ".txt"}
168-
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))))
168+
{:contents [{:type :text
169+
:error false
170+
:content (str (h/file-path "/project/foo/bar/baz.txt") "\n"
171+
(h/file-path "/project/foo/qux.txt"))}]}
172+
(with-redefs [fs/exists? (constantly true)
173+
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
174+
(fs/path (h/file-path "/project/foo/qux.txt"))])]
175+
((get-in f.tools.filesystem/definitions ["eca_search_files" :handler])
176+
{"path" (h/file-path "/project/foo")
177+
"pattern" ".txt"}
178+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))))
169179

170180
(deftest grep-test
171181
(testing "invalid pattern"

‎test/eca/shared_test.clj‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,18 @@
1515
(when h/windows? (shared/uri->filename "file:/c:/Users/FirstName%20LastName/c.clj"))))
1616
(is (= (when h/windows? "C:\\c.clj")
1717
(when h/windows? (shared/uri->filename "file:///c:/c.clj"))))))
18+
19+
(deftest assoc-some-test
20+
(testing "single association"
21+
(is (= {:a 1} (shared/assoc-some {} :a 1)))
22+
(is (= {} (shared/assoc-some {} :a nil))))
23+
(testing "multiple associations"
24+
(is (= {:a 1 :b 2}
25+
(shared/assoc-some {} :a 1 :b 2)))
26+
(is (= {:a 1}
27+
(shared/assoc-some {} :a 1 :b nil)))
28+
(is (= {}
29+
(shared/assoc-some {} :a nil :b nil))))
30+
(testing "throws on uneven kvs"
31+
(is (thrown? IllegalArgumentException
32+
(shared/assoc-some {} :a 1 :b)))))

0 commit comments

Comments
 (0)