Skip to content

Fix vendored/build path exclusion in TechDetector (closes #150) - #804

Open
DataMnk wants to merge 9 commits into
ascherj:mainfrom
DataMnk:fix/150-tech-detector-vendored-files
Open

Fix vendored/build path exclusion in TechDetector (closes #150)#804
DataMnk wants to merge 9 commits into
ascherj:mainfrom
DataMnk:fix/150-tech-detector-vendored-files

Conversation

@DataMnk

@DataMnk DataMnk commented Aug 4, 2026

Copy link
Copy Markdown

Summary

Fixes the TechDetector tool so it correctly excludes vendored/build files
(e.g. node_modules/, build/) when detecting a repository's primary
language, instead of incorrectly counting them and skewing the result
toward JavaScript.

Issue

Closes #150

Changes

  • Rewrote _should_skip_file() in agent/tools/tech_detector.py: instead
    of substring-matching slash-anchored patterns like "/node_modules/"
    (which silently failed to match relative paths without a leading slash,
    e.g. "node_modules/lib/index.js"), it now splits each filepath into
    path segments and checks whether any directory segment (excluding the
    filename itself) exactly matches a known vendor/build directory name
  • This correctly excludes paths with or without a leading slash, while
    avoiding false positives on filenames that merely contain a vendor
    directory name as a substring (e.g. src/node_modules_helper.py)

Testing

  • Unit tests pass (make test-unit)
  • Integration tests pass (make test-integration)
  • Linter passes (make lint) — verified on the modified file with
    ruff check agent/tools/tech_detector.py
  • Type checker passes (make typecheck)
  • New/updated tests cover the changes — the two previously-failing
    tests named in the issue, test_node_modules_excluded and
    test_build_directory_excluded, now pass. I also added three new
    regression tests for edge cases identified in my PLAN.md: a
    lookalike filename that should NOT be excluded
    (test_node_modules_lookalike_filename_not_excluded), a build/vendor.js
    path (test_build_vendor_file_excluded), and Windows backslash paths
    (test_windows_backslash_paths_excluded). Full file suite is now 30/30

To verify by hand:

  1. git checkout fix/150-tech-detector-vendored-files
  2. pytest tests/unit/test_tech_detector.py -v — all 30 tests pass
  3. In a Python REPL:
   from agent.tools.tech_detector import TechDetector
   files = ['main.py', 'node_modules/lib/index.js', 'build/bundle.js']
   TechDetector().execute({'files': files}).data['primary_language']
   # Returns 'Python' instead of 'JavaScript'

Screenshots / Demo

N/A — backend logic change, no UI impact.

Notes for Reviewers

When I first chose this bug, it took me some time to understand what it
was really about, so I started exploring the repo structure and files.
That took a while but there was a lot of new stuff to take in, so the
time was worth it. Once you understand it, it's pretty intuitive.

I used Claude to help explain the existing code and propose some fixes,
and this approach (splitting into path segments) was the one that made
the most sense to me — the best option out of what we considered.

There's always a lot to learn along the way, and it was tempting to look
into other bugs I noticed while exploring. That would have led to me not
delivering anything and just exploring endlessly, so I forced myself to
focus and just work on my bug.

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.

Tech detector counts vendored and build-output files, skewing language detection

1 participant