Skip to content

Commit a504891

Browse files
fix(fs): ls --simple skips abstract fetching and returns only URIs (#236)
When simple=True, short-circuit early using output='original' to avoid expensive abstract fetching (_batch_fetch_abstracts), then extract only the uri field. This fixes #218 where --simple with output=agent returned empty strings because agent format omits name/rel_path fields. Follows author guidance to save tokens by keeping only the uri column. Fixes #218
1 parent 4e0f35e commit a504891

File tree

3 files changed

+36
-4
lines changed

3 files changed

+36
-4
lines changed

openviking/service/fs_service.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,21 @@ async def ls(
5454
"""
5555
viking_fs = self._ensure_initialized()
5656

57+
if simple:
58+
# Only return URIs — skip expensive abstract fetching to save tokens
59+
if recursive:
60+
entries = await viking_fs.tree(
61+
uri,
62+
output="original",
63+
show_all_hidden=show_all_hidden,
64+
node_limit=node_limit,
65+
)
66+
else:
67+
entries = await viking_fs.ls(
68+
uri, output="original", show_all_hidden=show_all_hidden
69+
)
70+
return [e.get("uri", "") for e in entries]
71+
5772
if recursive:
5873
entries = await viking_fs.tree(
5974
uri,
@@ -66,9 +81,6 @@ async def ls(
6681
entries = await viking_fs.ls(
6782
uri, output=output, abs_limit=abs_limit, show_all_hidden=show_all_hidden
6883
)
69-
70-
if simple:
71-
return [e.get("rel_path", e.get("name", e.get("uri", ""))) for e in entries]
7284
return entries
7385

7486
async def mkdir(self, uri: str) -> None:

tests/client/test_filesystem.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,15 @@ async def test_ls_directory(self, client_with_resource):
2323
assert len(entries) > 0
2424

2525
async def test_ls_simple_mode(self, client_with_resource):
26-
"""Test simple mode listing"""
26+
"""Test simple mode listing returns non-empty URI strings (fixes #218)"""
2727
client, uri = client_with_resource
2828
parent_uri = "/".join(uri.split("/")[:-1]) + "/"
2929

3030
entries = await client.ls(parent_uri, simple=True)
3131

3232
assert isinstance(entries, list)
3333
assert all(isinstance(e, str) for e in entries)
34+
assert all(e.startswith("viking://") for e in entries)
3435

3536
async def test_ls_recursive(self, client_with_resource):
3637
"""Test recursive listing"""

tests/server/test_api_filesystem.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,25 @@ async def test_ls_simple(client: httpx.AsyncClient):
2525
body = resp.json()
2626
assert body["status"] == "ok"
2727
assert isinstance(body["result"], list)
28+
# Each item must be a non-empty URI string (fixes #218)
29+
for item in body["result"]:
30+
assert isinstance(item, str)
31+
assert item.startswith("viking://")
32+
33+
34+
async def test_ls_simple_agent_output(client: httpx.AsyncClient):
35+
"""Ensure --simple with output=agent returns URI strings, not empty."""
36+
resp = await client.get(
37+
"/api/v1/fs/ls",
38+
params={"uri": "viking://", "simple": True, "output": "agent"},
39+
)
40+
assert resp.status_code == 200
41+
body = resp.json()
42+
assert body["status"] == "ok"
43+
assert isinstance(body["result"], list)
44+
for item in body["result"]:
45+
assert isinstance(item, str)
46+
assert item.startswith("viking://")
2847

2948

3049
async def test_mkdir_and_ls(client: httpx.AsyncClient):

0 commit comments

Comments
 (0)