mirror of
https://github.com/VectifyAI/PageIndex.git
synced 2026-07-06 20:42:12 +02:00
fix(filesystem): enforce pifs shell command limits
This commit is contained in:
BukeLy
parent
cb9db0bab9
commit
9734bf6914
8 changed files with 780 additions and 29 deletions
|
|
@ -1,6 +1,8 @@
|
|||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class SummaryBackend:
|
||||
def __init__(self, document_id):
|
||||
|
|
@ -45,6 +47,32 @@ def test_semantic_search_scope_keeps_ordinary_folders_out_of_source_type_filters
|
|||
assert result["data"]["data"][0]["external_id"] == "dsid_report"
|
||||
|
||||
|
||||
def test_semantic_search_rejects_unquoted_multi_word_query(tmp_path):
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
|
||||
filesystem = PageIndexFileSystem(workspace=tmp_path / "workspace")
|
||||
filesystem.register_file(
|
||||
storage_uri="file:///tmp/report.pdf",
|
||||
source_path="examples/documents/report.pdf",
|
||||
folder_path="/documents",
|
||||
external_id="dsid_report",
|
||||
title="Annual report",
|
||||
content="Federal Reserve supervision and regulation annual report.",
|
||||
)
|
||||
filesystem.semantic_retrieval_backend = SummaryBackend("dsid_report")
|
||||
executor = PIFSCommandExecutor(filesystem, json_output=True)
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="Quote multi-word queries"):
|
||||
executor.execute("search-summary Federal Reserve /documents")
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="quote it"):
|
||||
executor.execute("search-summary Federal Reserve")
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="does not support regex alternation"):
|
||||
executor.execute('search-summary "Federal|Reserve" /documents')
|
||||
|
||||
|
||||
def test_semantic_search_scope_filters_explicit_source_type_facets():
|
||||
from pageindex.filesystem import PageIndexFileSystem
|
||||
|
||||
|
|
|
|||
|
|
@ -341,8 +341,10 @@ def test_cat_structure_page_reuses_pageindex_client_cache_without_indexing(monke
|
|||
assert structure["data"]["available"] is True
|
||||
assert structure["data"]["pageindex_doc_id"] == "doc_cached_pdf"
|
||||
assert structure["data"]["structure"][0]["title"] == "Introduction"
|
||||
assert structure["data"]["structure"][1]["title"] == "Findings"
|
||||
assert structure["data"]["structure_pagination"]["limit"] == 25
|
||||
assert "text" not in structure["data"]["structure"][0]
|
||||
assert "text" not in structure["data"]["structure"][0]["nodes"][0]
|
||||
assert "text" not in structure["data"]["structure"][1]
|
||||
|
||||
assert pages["data"]["available"] is True
|
||||
assert pages["data"]["text"] == "Page one text\n\nPage two text"
|
||||
|
|
@ -401,6 +403,92 @@ def test_cat_node_reads_pageindex_client_structure_without_custom_pifs_artifact(
|
|||
assert "text" not in node["data"]["node"]
|
||||
|
||||
|
||||
def test_cat_structure_page_node_and_text_outputs_are_hard_limited():
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
source = Path(tmp) / "report.pdf"
|
||||
source.write_bytes(b"%PDF-1.4\n% test fixture\n")
|
||||
filesystem = PageIndexFileSystem(workspace=Path(tmp) / "workspace")
|
||||
structure_nodes = [
|
||||
{
|
||||
"title": f"Section {index}",
|
||||
"node_id": f"{index:04d}",
|
||||
"start_index": index,
|
||||
"end_index": index,
|
||||
"text": f"node {index} text",
|
||||
"nodes": [],
|
||||
}
|
||||
for index in range(1, 31)
|
||||
]
|
||||
write_pageindex_client_doc(
|
||||
filesystem.pageindex_client_workspace,
|
||||
"doc_limited_pdf",
|
||||
{
|
||||
"id": "doc_limited_pdf",
|
||||
"type": "pdf",
|
||||
"path": str(source.resolve()),
|
||||
"doc_name": "report.pdf",
|
||||
"doc_description": "",
|
||||
"page_count": 10,
|
||||
"structure": structure_nodes,
|
||||
"pages": [
|
||||
{"page": index, "content": f"Page {index} text"}
|
||||
for index in range(1, 11)
|
||||
],
|
||||
},
|
||||
)
|
||||
filesystem.register_file(
|
||||
storage_uri=source.as_uri(),
|
||||
source_path="docs/report.pdf",
|
||||
external_id="dsid_limited_pdf",
|
||||
title="Limited structural report",
|
||||
content="text artifact remains available for grep",
|
||||
)
|
||||
text_content = "\n".join(f"line {index}" for index in range(1, 106))
|
||||
filesystem.register_file(
|
||||
storage_uri="file:///tmp/long.txt",
|
||||
source_path="docs/long.txt",
|
||||
external_id="dsid_long_text",
|
||||
title="Long text",
|
||||
content=text_content,
|
||||
)
|
||||
executor = PIFSCommandExecutor(filesystem, json_output=True)
|
||||
|
||||
first_structure = json.loads(executor.execute("cat dsid_limited_pdf --structure"))
|
||||
assert len(first_structure["data"]["structure"]) == 25
|
||||
assert first_structure["data"]["structure_pagination"]["has_more"] is True
|
||||
assert first_structure["data"]["structure_pagination"]["next_offset"] == 25
|
||||
|
||||
second_structure = json.loads(
|
||||
executor.execute("cat dsid_limited_pdf --structure --offset 25")
|
||||
)
|
||||
assert len(second_structure["data"]["structure"]) == 5
|
||||
assert second_structure["data"]["structure"][0]["node_id"] == "0026"
|
||||
|
||||
pages = json.loads(executor.execute("cat dsid_limited_pdf --page 1-3"))
|
||||
assert pages["data"]["text"] == "Page 1 text\n\nPage 2 text\n\nPage 3 text"
|
||||
assert pages["data"]["page_pagination"]["limit"] == 3
|
||||
with pytest.raises(PIFSCommandError, match="at most 3"):
|
||||
executor.execute("cat dsid_limited_pdf --page 1-4")
|
||||
|
||||
nodes = json.loads(
|
||||
executor.execute("cat dsid_limited_pdf --node 0001,0002,0003,0004,0005")
|
||||
)
|
||||
assert nodes["data"]["node_ids"] == ["0001", "0002", "0003", "0004", "0005"]
|
||||
with pytest.raises(PIFSCommandError, match="at most 5"):
|
||||
executor.execute("cat dsid_limited_pdf --node 0001,0002,0003,0004,0005,0006")
|
||||
|
||||
text = json.loads(executor.execute("cat dsid_long_text --all"))
|
||||
assert "line 100" in text["data"]["text"]
|
||||
assert "line 101" not in text["data"]["text"]
|
||||
assert text["data"]["pagination"]["has_more"] is True
|
||||
assert text["data"]["pagination"]["next_range"] == "101-105"
|
||||
with pytest.raises(PIFSCommandError, match="at most 100"):
|
||||
executor.execute("cat dsid_long_text --range 1-101")
|
||||
|
||||
|
||||
def test_tree_folder_behavior_is_preserved():
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
|
||||
|
|
|
|||
|
|
@ -207,10 +207,14 @@ class PIFSAgentStreamTest(unittest.TestCase):
|
|||
self.assertIn("stat --schema and stat <target>", AGENT_TOOL_POLICY)
|
||||
self.assertIn("do not infer metadata presence or absence", AGENT_TOOL_POLICY)
|
||||
self.assertIn("questions about metadata fields", BASH_TOOL_DESCRIPTION)
|
||||
self.assertIn("Use stat only for metadata/schema/status questions", AGENT_TOOL_POLICY)
|
||||
self.assertIn("Do not run stat merely to understand what a document says", AGENT_TOOL_POLICY)
|
||||
self.assertIn("Do not use stat as a general content/topic discovery step", BASH_TOOL_DESCRIPTION)
|
||||
|
||||
def test_prompt_routes_summary_search_to_search_summary(self):
|
||||
self.assertIn("search-summary when the user asks for", BASH_TOOL_DESCRIPTION)
|
||||
self.assertIn("use search-summary <query> <folder>", AGENT_TOOL_POLICY)
|
||||
self.assertIn('use search-summary "<query>" <folder>', AGENT_TOOL_POLICY)
|
||||
self.assertIn('search-summary "Federal Reserve" /documents', BASH_TOOL_DESCRIPTION)
|
||||
self.assertIn("do not translate that request into find --where", AGENT_TOOL_POLICY)
|
||||
|
||||
def test_system_prompt_sets_workspace_identity_and_scope(self):
|
||||
|
|
@ -222,6 +226,8 @@ class PIFSAgentStreamTest(unittest.TestCase):
|
|||
self.assertIn("workspace-related topic question", AGENT_SYSTEM_PROMPT)
|
||||
self.assertIn("clarify only after a reasonable search", AGENT_SYSTEM_PROMPT)
|
||||
self.assertIn("search for candidate documents before asking", AGENT_TOOL_POLICY)
|
||||
self.assertIn("Do not conclude that no relevant document exists from one failed grep", AGENT_SYSTEM_PROMPT)
|
||||
self.assertIn("A single failed grep is not enough evidence", AGENT_TOOL_POLICY)
|
||||
|
||||
def test_threaded_runtime_error_is_not_retried_on_fresh_loop(self):
|
||||
session = object.__new__(PIFSAgentSession)
|
||||
|
|
|
|||
|
|
@ -98,6 +98,37 @@ def test_stable_path_targets_work_without_session_refs(tmp_path):
|
|||
assert "Root document fixture text" in text
|
||||
|
||||
|
||||
def test_shell_limits_reject_context_expanding_counts(tmp_path):
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
|
||||
executor = _register_find_fixture(tmp_path)
|
||||
|
||||
for command, limit in (
|
||||
("find /documents --limit 51", 50),
|
||||
("grep --limit 21 Root /documents", 20),
|
||||
("ls /documents --limit 101", 100),
|
||||
("tree /documents --limit 201", 200),
|
||||
("head -n 101 /documents/Root\\ document", 100),
|
||||
("tail -n 101 /documents/Root\\ document", 100),
|
||||
("sed -n 1,101p /documents/Root\\ document", 100),
|
||||
):
|
||||
with pytest.raises(PIFSCommandError, match=f"at most {limit}"):
|
||||
executor.execute(command)
|
||||
|
||||
|
||||
def test_grep_rejects_regex_alternation_patterns(tmp_path):
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
|
||||
executor = _register_find_fixture(tmp_path)
|
||||
executor.json_output = False
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="does not support regex alternation"):
|
||||
executor.execute('grep -R "Root|Child" /documents')
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="multiple grep commands"):
|
||||
executor.execute('find /documents -type f | grep "Root|Child"')
|
||||
|
||||
|
||||
def test_stat_shell_output_includes_unified_metadata_status(tmp_path):
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
from pageindex.filesystem.metadata_generation import MetadataGenerationResult
|
||||
|
|
@ -142,6 +173,99 @@ def test_stat_shell_output_includes_unified_metadata_status(tmp_path):
|
|||
assert "metadata_status: generated" in stat
|
||||
|
||||
|
||||
def test_stat_field_reads_one_metadata_field_across_multiple_targets(tmp_path):
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
from pageindex.filesystem.metadata_generation import MetadataGenerationResult
|
||||
|
||||
class SummaryGenerator:
|
||||
def generate(self, document, *, fields):
|
||||
return MetadataGenerationResult(
|
||||
values={
|
||||
field: (
|
||||
f"Summary for {document.title}\n"
|
||||
+ "full summary token " * 80
|
||||
)
|
||||
for field in fields
|
||||
}
|
||||
)
|
||||
|
||||
filesystem = PageIndexFileSystem(
|
||||
workspace=tmp_path / "workspace",
|
||||
metadata_generator=SummaryGenerator(),
|
||||
)
|
||||
for index in range(1, 3):
|
||||
source = tmp_path / f"source{index}.txt"
|
||||
source.write_text(f"fixture text {index}", encoding="utf-8")
|
||||
filesystem.register_file(
|
||||
storage_uri=source.as_uri(),
|
||||
source_path=f"docs/source{index}.txt",
|
||||
folder_path="/documents",
|
||||
external_id=f"doc_summary_{index}",
|
||||
title=f"Summary document {index}",
|
||||
content=source.read_text(encoding="utf-8"),
|
||||
metadata_policy={
|
||||
"fields": {
|
||||
"summary": True,
|
||||
"doc_type": False,
|
||||
"domain": False,
|
||||
"topic": False,
|
||||
}
|
||||
},
|
||||
)
|
||||
executor = PIFSCommandExecutor(filesystem, json_output=False)
|
||||
|
||||
output = executor.execute(
|
||||
"stat --field summary /documents/'Summary document 1' /documents/'Summary document 2'"
|
||||
)
|
||||
|
||||
assert "/documents/Summary document 1:" in output
|
||||
assert "summary: Summary for Summary document 1" in output
|
||||
assert "full summary token" in output
|
||||
assert "[truncated]" not in output
|
||||
assert "/documents/Summary document 2:" in output
|
||||
assert "summary: Summary for Summary document 2" in output
|
||||
|
||||
data = json.loads(
|
||||
PIFSCommandExecutor(filesystem, json_output=True).execute(
|
||||
"stat --field summary /documents/'Summary document 1' /documents/'Summary document 2'"
|
||||
)
|
||||
)["data"]
|
||||
assert data["mode"] == "field_values"
|
||||
assert data["target_count"] == 2
|
||||
assert data["data"][0]["field"] == "summary"
|
||||
assert data["data"][0]["value"].startswith("Summary for Summary document 1\n")
|
||||
assert data["data"][0]["value"].count("full summary token") == 80
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="Unknown metadata field"):
|
||||
executor.execute("stat --field missing_field /documents/'Summary document 1'")
|
||||
|
||||
|
||||
def test_stat_field_rejects_more_than_twenty_targets(tmp_path):
|
||||
from pageindex.filesystem import PIFSCommandExecutor, PageIndexFileSystem
|
||||
from pageindex.filesystem.commands import PIFSCommandError
|
||||
|
||||
filesystem = PageIndexFileSystem(workspace=tmp_path / "workspace")
|
||||
targets = []
|
||||
for index in range(21):
|
||||
source = tmp_path / f"source{index}.txt"
|
||||
source.write_text(f"fixture text {index}", encoding="utf-8")
|
||||
filesystem.register_file(
|
||||
storage_uri=source.as_uri(),
|
||||
source_path=f"docs/source{index}.txt",
|
||||
folder_path="/documents",
|
||||
external_id=f"doc_{index}",
|
||||
title=f"Document {index}",
|
||||
content=source.read_text(encoding="utf-8"),
|
||||
metadata={"department": "ops"},
|
||||
)
|
||||
targets.append(f"/documents/'Document {index}'")
|
||||
executor = PIFSCommandExecutor(filesystem, json_output=False)
|
||||
|
||||
with pytest.raises(PIFSCommandError, match="at most 20"):
|
||||
executor.execute("stat --field department " + " ".join(targets))
|
||||
|
||||
|
||||
def test_register_rejects_pifs_owned_metadata_fields(tmp_path):
|
||||
from pageindex.filesystem import PageIndexFileSystem
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue