mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
67db3d883c
* fix(middlewares): include file line count in read-before-write block message and clarify read_file range error * fix(middlewares): address review feedback on line count pluralization and readability * fix(middlewares): address round-2 review feedback for PR #6020 Stamping bug (_attach_read_mark, ggnnggez item 1): Originally planned as a standalone issue; folded here since it is the exact mechanism behind the #6019 gate bypass. _attach_read_mark now returns early when message.content is in READ_FILE_NO_CONTENT_RESULTS - {READ_FILE_EMPTY}. In-band errors (invalid range, start_line exceeds file length, etc.) are returned with status='success' by LangChain so the existing message.status == 'error' guard did not catch them. HTTP code collision [P3] (willem-bd): Pre-stamp TOOL_META_KEY on the blocked ToolMessage before normalize_tool_result so files with 401, 403, 404, or 500 lines are not misclassified as auth/server errors by _classify_error_text. normalize_tool_message returns early when an existing TOOL_META_KEY entry is found. Line-count parity (ggnnggez, inline L317): Replace splitlines() with count_file_lines() in read_file_contract.py using count('\n'), matching LocalSandbox text-mode iteration and the _truncate_read_file_output formula. splitlines() over-counts for files with \f or \u2028 separators (e.g. pdftotext output). Read hint (ggnnggez, inline _BLOCK_MESSAGE): For empty files the ranged-read suggestion is omitted. For non-empty files the hint gives a concrete start_line=max(1, N-29), end_line=N. Constant rename (ggnnggez, read_file_contract.py): READ_FILE_INVALID_RANGE is the new canonical name; READ_FILE_EMPTY_RANGE kept as a backward-compat alias. _LEGACY_READ_FILE_EMPTY_RANGE removed (unreachable after the #6019 contract update). Tests: - TestBlockMessageLineCount parametrized with \f and \u2028 parity cases - TestStampingGateBugFix: 4 no-content variants, gate-closed integration probe, empty-file-still-stamps assertion - test_gate_block_on_numeric_line_count_is_recoverable: 401/403/404/500 - test_contract_constants_membership replaced with test_contract_constants_parity_with_localsandbox --- NOT INCLUDED: ggnnggez item 2 (file length in read_file range error strings). READ_FILE_NO_CONTENT_RESULTS uses exact string membership and is also consumed by build_skill_usage for skill-read detection; making those strings dynamic requires a dedicated contract refactor. Deferred to a follow-up PR. Note: test_web_fetch_relative_links failures in the full suite are pre-existing on upstream/main and unrelated to this change. * test: clarify blank ranged reads in the write gate --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
124 lines
4.7 KiB
Python
124 lines
4.7 KiB
Python
"""read_file tool line-range handling for one-sided (single-bound) ranges.
|
|
|
|
Previously ``read_file`` only sliced when BOTH ``start_line`` and ``end_line``
|
|
were supplied; a lone ``start_line`` (or lone ``end_line``) was silently ignored
|
|
and the whole file was returned. These tests pin the one-sided range contract:
|
|
tail-from-start, head-to-end, rejection of non-positive line numbers, and clean
|
|
error strings for an inverted range or a start beyond EOF (instead of an
|
|
empty/garbage slice).
|
|
"""
|
|
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
|
|
from deerflow.sandbox.local.local_sandbox import LocalSandbox
|
|
from deerflow.sandbox.tools import read_file_tool
|
|
|
|
_FIVE_LINES = "line1\nline2\nline3\nline4\nline5"
|
|
|
|
|
|
def _local_runtime(tmp_path: Path) -> SimpleNamespace:
|
|
for sub in ("workspace", "uploads", "outputs"):
|
|
(tmp_path / sub).mkdir(parents=True, exist_ok=True)
|
|
thread_data = {
|
|
"workspace_path": str(tmp_path / "workspace"),
|
|
"uploads_path": str(tmp_path / "uploads"),
|
|
"outputs_path": str(tmp_path / "outputs"),
|
|
}
|
|
return SimpleNamespace(
|
|
state={"sandbox": {"sandbox_id": "local:t1"}, "thread_data": thread_data},
|
|
context={"thread_id": "t1"},
|
|
)
|
|
|
|
|
|
def _read(tmp_path, monkeypatch, **kwargs) -> str:
|
|
runtime = _local_runtime(tmp_path)
|
|
(tmp_path / "uploads" / "five.txt").write_text(_FIVE_LINES, encoding="utf-8")
|
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: LocalSandbox("t1"))
|
|
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
|
|
return read_file_tool.func(
|
|
runtime=runtime,
|
|
description="read a line range",
|
|
path="/mnt/user-data/uploads/five.txt",
|
|
**kwargs,
|
|
)
|
|
|
|
|
|
def test_only_start_line_returns_tail_from_that_line(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, start_line=3)
|
|
assert result == "line3\nline4\nline5"
|
|
|
|
|
|
def test_only_end_line_returns_head_up_to_that_line(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, end_line=2)
|
|
assert result == "line1\nline2"
|
|
|
|
|
|
def test_start_line_zero_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, start_line=0)
|
|
assert "start_line must be >= 1" in result
|
|
assert "line1" not in result
|
|
|
|
|
|
def test_start_line_negative_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, start_line=-1)
|
|
assert "start_line must be >= 1" in result
|
|
assert "line5" not in result
|
|
|
|
|
|
def test_start_line_greater_than_end_line_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
from deerflow.sandbox.read_file_contract import READ_FILE_EMPTY_RANGE
|
|
|
|
result = _read(tmp_path, monkeypatch, start_line=4, end_line=2)
|
|
assert result == READ_FILE_EMPTY_RANGE
|
|
# No garbage slice content leaked into the error.
|
|
assert "line4" not in result
|
|
|
|
|
|
def test_start_line_beyond_eof_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, start_line=99)
|
|
assert "start_line exceeds file length" in result
|
|
|
|
|
|
def test_both_bounds_still_slice_inclusive_range(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, start_line=2, end_line=4)
|
|
assert result == "line2\nline3\nline4"
|
|
|
|
|
|
def test_only_end_line_zero_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, end_line=0)
|
|
assert "end_line must be >= 1" in result
|
|
# No leaked line content in the error.
|
|
assert "line1" not in result
|
|
|
|
|
|
def test_only_end_line_negative_returns_clean_error(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, end_line=-1)
|
|
assert "end_line must be >= 1" in result
|
|
assert "line4" not in result
|
|
|
|
|
|
def test_end_line_past_eof_clamps_to_last_line(tmp_path, monkeypatch) -> None:
|
|
result = _read(tmp_path, monkeypatch, end_line=99)
|
|
assert result == _FIVE_LINES
|
|
|
|
|
|
def test_contract_constants_parity_with_localsandbox() -> None:
|
|
"""count_file_lines must agree with LocalSandbox line numbering and all no-content
|
|
strings must remain in READ_FILE_NO_CONTENT_RESULTS."""
|
|
from deerflow.sandbox.read_file_contract import (
|
|
READ_FILE_INVALID_RANGE,
|
|
READ_FILE_NO_CONTENT_RESULTS,
|
|
count_file_lines,
|
|
)
|
|
|
|
assert READ_FILE_INVALID_RANGE in READ_FILE_NO_CONTENT_RESULTS
|
|
|
|
# Parity with LocalSandbox.read_file (Python text-mode, universal \n only):
|
|
assert count_file_lines("") == 0
|
|
assert count_file_lines("line1\n") == 1
|
|
assert count_file_lines("line1\nline2\n") == 2
|
|
assert count_file_lines("line1\nline2") == 2 # no trailing newline
|
|
assert count_file_lines("page\fend\n") == 1 # \f not a line separator
|
|
assert count_file_lines("x\u2028y\n") == 1 # \u2028 not a line separator
|