Skip to content

Commit 336bd62

Browse files
Hi, Jules here! I've gone ahead and improved the code health by deduplicating the snapshot loading logic.
I extracted the duplicate snapshot parsing logic from `src/tools.py` and `src/commands.py` and moved it into a shared `load_porting_modules` utility function within a new `src/snapshot_loader.py` file. This reduces duplicate code and significantly improves maintainability. Going forward, any changes to how snapshots are loaded or how `PortingModule` objects are constructed from them can now be managed in a single place! Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent efd7cfd commit 336bd62

3 files changed

Lines changed: 68 additions & 37 deletions

File tree

src/commands.py

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
from __future__ import annotations
22

3-
import json
43
from collections.abc import Mapping
54
from dataclasses import dataclass
65
from functools import lru_cache
76
from pathlib import Path
87
from types import MappingProxyType
98

109
from .models import PortingBacklog, PortingModule
10+
from .snapshot_loader import load_porting_modules
1111

1212
SNAPSHOT_PATH = (
1313
Path(__file__).resolve().parent / "reference_data" / "commands_snapshot.json"
@@ -25,16 +25,7 @@ class CommandExecution:
2525

2626
@lru_cache(maxsize=1)
2727
def load_command_snapshot() -> tuple[PortingModule, ...]:
28-
raw_entries = json.loads(SNAPSHOT_PATH.read_text())
29-
return tuple(
30-
PortingModule(
31-
name=entry["name"],
32-
responsibility=entry["responsibility"],
33-
source_hint=entry["source_hint"],
34-
status="mirrored",
35-
)
36-
for entry in raw_entries
37-
)
28+
return load_porting_modules(SNAPSHOT_PATH)
3829

3930

4031
PORTED_COMMANDS = load_command_snapshot()

src/snapshot_loader.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import json
2+
from pathlib import Path
3+
4+
from .models import PortingModule
5+
6+
7+
def load_porting_modules(snapshot_path: Path) -> tuple[PortingModule, ...]:
8+
raw_entries = json.loads(snapshot_path.read_text())
9+
return tuple(
10+
PortingModule(
11+
name=entry["name"],
12+
responsibility=entry["responsibility"],
13+
source_hint=entry["source_hint"],
14+
status="mirrored",
15+
)
16+
for entry in raw_entries
17+
)

src/tools.py

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
from __future__ import annotations
22

3-
import json
43
from dataclasses import dataclass
54
from functools import lru_cache
65
from pathlib import Path
76

87
from .models import PortingBacklog, PortingModule
8+
from .snapshot_loader import load_porting_modules
99
from .permissions import ToolPermissionContext
1010

11-
SNAPSHOT_PATH = Path(__file__).resolve().parent / 'reference_data' / 'tools_snapshot.json'
11+
SNAPSHOT_PATH = (
12+
Path(__file__).resolve().parent / "reference_data" / "tools_snapshot.json"
13+
)
1214

1315

1416
@dataclass(frozen=True)
@@ -22,23 +24,14 @@ class ToolExecution:
2224

2325
@lru_cache(maxsize=1)
2426
def load_tool_snapshot() -> tuple[PortingModule, ...]:
25-
raw_entries = json.loads(SNAPSHOT_PATH.read_text())
26-
return tuple(
27-
PortingModule(
28-
name=entry['name'],
29-
responsibility=entry['responsibility'],
30-
source_hint=entry['source_hint'],
31-
status='mirrored',
32-
)
33-
for entry in raw_entries
34-
)
27+
return load_porting_modules(SNAPSHOT_PATH)
3528

3629

3730
PORTED_TOOLS = load_tool_snapshot()
3831

3932

4033
def build_tool_backlog() -> PortingBacklog:
41-
return PortingBacklog(title='Tool surface', modules=list(PORTED_TOOLS))
34+
return PortingBacklog(title="Tool surface", modules=list(PORTED_TOOLS))
4235

4336

4437
def tool_names() -> list[str]:
@@ -53,10 +46,15 @@ def get_tool(name: str) -> PortingModule | None:
5346
return None
5447

5548

56-
def filter_tools_by_permission_context(tools: tuple[PortingModule, ...], permission_context: ToolPermissionContext | None = None) -> tuple[PortingModule, ...]:
49+
def filter_tools_by_permission_context(
50+
tools: tuple[PortingModule, ...],
51+
permission_context: ToolPermissionContext | None = None,
52+
) -> tuple[PortingModule, ...]:
5753
if permission_context is None:
5854
return tools
59-
return tuple(module for module in tools if not permission_context.blocks(module.name))
55+
return tuple(
56+
module for module in tools if not permission_context.blocks(module.name)
57+
)
6058

6159

6260
def get_tools(
@@ -66,31 +64,56 @@ def get_tools(
6664
) -> tuple[PortingModule, ...]:
6765
tools = list(PORTED_TOOLS)
6866
if simple_mode:
69-
tools = [module for module in tools if module.name in {'BashTool', 'FileReadTool', 'FileEditTool'}]
67+
tools = [
68+
module
69+
for module in tools
70+
if module.name in {"BashTool", "FileReadTool", "FileEditTool"}
71+
]
7072
if not include_mcp:
71-
tools = [module for module in tools if 'mcp' not in module.name.lower() and 'mcp' not in module.source_hint.lower()]
73+
tools = [
74+
module
75+
for module in tools
76+
if "mcp" not in module.name.lower()
77+
and "mcp" not in module.source_hint.lower()
78+
]
7279
return filter_tools_by_permission_context(tuple(tools), permission_context)
7380

7481

7582
def find_tools(query: str, limit: int = 20) -> list[PortingModule]:
7683
needle = query.lower()
77-
matches = [module for module in PORTED_TOOLS if needle in module.name.lower() or needle in module.source_hint.lower()]
84+
matches = [
85+
module
86+
for module in PORTED_TOOLS
87+
if needle in module.name.lower() or needle in module.source_hint.lower()
88+
]
7889
return matches[:limit]
7990

8091

81-
def execute_tool(name: str, payload: str = '') -> ToolExecution:
92+
def execute_tool(name: str, payload: str = "") -> ToolExecution:
8293
module = get_tool(name)
8394
if module is None:
84-
return ToolExecution(name=name, source_hint='', payload=payload, handled=False, message=f'Unknown mirrored tool: {name}')
95+
return ToolExecution(
96+
name=name,
97+
source_hint="",
98+
payload=payload,
99+
handled=False,
100+
message=f"Unknown mirrored tool: {name}",
101+
)
85102
action = f"Mirrored tool '{module.name}' from {module.source_hint} would handle payload {payload!r}."
86-
return ToolExecution(name=module.name, source_hint=module.source_hint, payload=payload, handled=True, message=action)
103+
return ToolExecution(
104+
name=module.name,
105+
source_hint=module.source_hint,
106+
payload=payload,
107+
handled=True,
108+
message=action,
109+
)
87110

88111

89112
def render_tool_index(limit: int = 20, query: str | None = None) -> str:
90113
modules = find_tools(query, limit) if query else list(PORTED_TOOLS[:limit])
91-
lines = [f'Tool entries: {len(PORTED_TOOLS)}', '']
114+
lines = [f"Tool entries: {len(PORTED_TOOLS)}", ""]
92115
if query:
93-
lines.append(f'Filtered by: {query}')
94-
lines.append('')
95-
lines.extend(f'- {module.name}{module.source_hint}' for module in modules)
96-
return '\n'.join(lines)
116+
lines.append(f"Filtered by: {query}")
117+
lines.append("")
118+
lines.extend(f"- {module.name}{module.source_hint}" for module in modules)
119+
return "\n".join(lines)

0 commit comments

Comments
 (0)