Skip to content

Commit 0fa6f6d

Browse files
committed
fix: review findings -- path-aware include_paths, broader import extensions, requirements
1. include_paths: replaced string startswith with Path.parts comparison to prevent 'src/app' matching 'src/application' (new test added) 2. Absolute import resolver: extended extensions from [.py, .js, .ts] to match relative resolver ['', .ts, .tsx, .d.ts, .js, .jsx, .py] 3. Added tree-sitter-typescript to requirements.txt for CI 4. Tests: replaced unused option_deps with assertion, added ./types.js assertion to tsx_imports test, added prefix confusion test Note: callers in routes/analysis.py still call build_dependency_graph without include_paths because include_paths is not persisted to the repositories table. Tracked as separate schema change.
1 parent d3e7932 commit 0fa6f6d

3 files changed

Lines changed: 23 additions & 4 deletions

File tree

backend/requirements.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ hiredis>=2.3.0
1818
tree-sitter>=0.23.0
1919
tree-sitter-python>=0.23.0
2020
tree-sitter-javascript>=0.23.0
21+
tree-sitter-typescript>=0.23.0
2122

2223
# AI/ML
2324
openai>=1.54.0

backend/services/dependency_analyzer.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,11 @@ def build_dependency_graph(self, repo_path: str, include_paths: List[str] = None
141141
if file_path.suffix not in extensions:
142142
continue
143143
if include_paths:
144-
relative = str(file_path.relative_to(repo_path))
145-
if not any(relative.startswith(p) for p in include_paths):
144+
rel_parts = file_path.relative_to(repo_path).parts
145+
if not any(
146+
rel_parts[:len(Path(p).parts)] == Path(p).parts
147+
for p in include_paths
148+
):
146149
continue
147150
code_files.append(file_path)
148151

@@ -280,7 +283,7 @@ def _resolve_import_to_file(
280283
if not import_path.startswith('.'):
281284
module_path = import_path.replace('.', '/')
282285

283-
for ext in ['.py', '.js', '.ts']:
286+
for ext in ['', '.ts', '.tsx', '.d.ts', '.js', '.jsx', '.py']:
284287
test_path = module_path + ext
285288
if test_path in internal_files:
286289
return test_path

backend/tests/test_dependency_analyzer.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ def test_tsx_imports(self, analyzer, tsx_repo):
165165
imports = set(result['imports'])
166166
assert "react" in imports
167167
assert "../utils.js" in imports
168+
assert "./types.js" in imports
168169

169170
def test_import_count(self, analyzer, ts_repo):
170171
result = analyzer.analyze_file_dependencies(
@@ -180,8 +181,8 @@ def test_js_extension_resolves_to_ts(self, analyzer, ts_repo):
180181
"""import from './Function.js' should resolve to Function.ts"""
181182
graph = analyzer.build_dependency_graph(str(ts_repo))
182183
deps = graph['dependencies']
184+
assert 'packages/effect/src/Option.ts' in deps
183185

184-
option_deps = deps.get('packages/effect/src/Option.ts', [])
185186
# Function.js should resolve to Function.ts
186187
resolved_targets = set()
187188
for edge in graph['edges']:
@@ -226,6 +227,20 @@ def test_include_paths_filters_to_subset(self, analyzer, ts_repo):
226227
# Should NOT have schema files
227228
assert not any('packages/schema' in f for f in file_paths)
228229

230+
def test_include_paths_no_prefix_confusion(self, analyzer, tmp_path):
231+
"""'src/app' must not match 'src/application'"""
232+
(tmp_path / "src" / "app").mkdir(parents=True)
233+
(tmp_path / "src" / "application").mkdir(parents=True)
234+
(tmp_path / "src" / "app" / "index.ts").write_text('export const x = 1')
235+
(tmp_path / "src" / "application" / "index.ts").write_text('export const y = 2')
236+
237+
graph = analyzer.build_dependency_graph(
238+
str(tmp_path), include_paths=['src/app']
239+
)
240+
file_paths = set(graph['dependencies'].keys())
241+
assert any('src/app/index.ts' in f for f in file_paths)
242+
assert not any('src/application' in f for f in file_paths)
243+
229244
def test_include_paths_multiple_dirs(self, analyzer, ts_repo):
230245
"""Multiple include_paths should include all specified dirs"""
231246
graph = analyzer.build_dependency_graph(

0 commit comments

Comments
 (0)