Skip to content

Graphcore changes for natspec/autoprove#18

Open
jtoman wants to merge 3 commits into
masterfrom
jtoman/fs-layer-updates
Open

Graphcore changes for natspec/autoprove#18
jtoman wants to merge 3 commits into
masterfrom
jtoman/fs-layer-updates

Conversation

@jtoman
Copy link
Copy Markdown
Contributor

@jtoman jtoman commented May 20, 2026

  1. Make the memory tool resilient to malformed paths, and not crash the whole pipeline
  2. FS tool expansion
    1. Allows writing exclusion rules for FS/VFS tools using a lambda instead of a regex (what WAS I thinking?)
    2. Adds an "edit file" tool, modeled after the str_replace used by claude code, codex, etc. Much more token efficient for targeted edits
    3. Adds a notion of "global exclude", a predicate where the files are not just hidden from tools, but materialization. This is useful for when you don't want the agent to read/write the .git directory or copy its contents during materialization
    4. Adds a "layered" fs tools abstraction, effectively a union FS implement via FSBackends. The current fs_tools is simply an instantiation of this new layered abstraction with a single "DirBackend".

@jtoman jtoman requested a review from naftali-g May 20, 2026 23:08
Comment thread graphcore/tools/vfs.py


# Type alias for the user-facing ``global_exclude`` config. Both forms
# return True to *exclude* the path. The callable form (preferred) is
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's confusing that _floor_include returns True if the path should be included, but the exclude returns True in the opposite case.

Comment thread graphcore/tools/vfs.py
Comment on lines +518 to +521
if replace_all:
new_content = cont.replace(old_string, new_string)
else:
new_content = cont.replace(old_string, new_string, 1)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically you don't need this if-else - you already know that there's just 1 occurrence when replace_all is False.

Comment thread graphcore/tools/vfs.py
return tool_output(
tool_call_id=tool_call_id,
res={
"vfs": {norm_path: new_content},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this tool saves on the number of tokens the llm produces to write the file, but it will still get the whole new file in the response?

Comment thread graphcore/tools/vfs.py
if cache_listing:
_list_all_files = cache(_list_all_files)
@dataclass
class DirBackend:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't want this class to explicitly inherit FSBackend?

Comment thread uv.lock
{ name = "langchain-anthropic", specifier = ">=1.4" },
{ name = "langchain-core", specifier = ">=1.2" },
{ name = "langgraph", specifier = "==1.1.0" },
{ name = "langgraph", specifier = "==1.0.5" },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

purposely downgrading?

Comment thread uv.lock
[[package]]
name = "langchain"
version = "1.2.11"
version = "1.2.6"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants