Skip to content

Fix: tech detector excludes vendored/build files only when nested, not at path root (#150) - #1019

Open
sojsun17 wants to merge 7 commits into
ascherj:mainfrom
sojsun17:fix/150-tech-detector-exclude-paths
Open

Fix: tech detector excludes vendored/build files only when nested, not at path root (#150)#1019
sojsun17 wants to merge 7 commits into
ascherj:mainfrom
sojsun17:fix/150-tech-detector-exclude-paths

Conversation

@sojsun17

@sojsun17 sojsun17 commented Aug 9, 2026

Copy link
Copy Markdown

Summary

Fixes _should_skip_file() in agent/tools/tech_detector.py so vendored and
build-output directories are correctly excluded from language detection
regardless of whether they appear at the root of a file path or nested
deeper. The previous implementation matched skip-directories as raw
substrings requiring a leading slash (e.g. "/build/" in filepath), which
missed root-level paths like "build/bundle.js" or "node_modules/x.js"
that have no parent directory — causing repos that are mostly one language
to be misclassified as another based on bundled/vendored files.

Issue

Closes #150

Changes

  • Replaced substring matching in _should_skip_file() with path-segment
    matching: split each path on / and check whether any individual
    segment exactly matches a skip-directory name (node_modules, vendor,
    dist, build, .git, __pycache__, .venv, venv).
  • This fixes root-level skip-directories while preserving correct behavior
    for nested ones, and avoids false positives on filenames that merely
    contain a skip-dir word as a substring (e.g. src/rebuild/utils.py,
    vendor_utils.py).
  • Added 4 new regression tests: root-level node_modules/build exclusion,
    filename-substring false-positive check, and an all-vendored-files case
    that should resolve to "Unknown".
  • Added missing assertions to 5 existing tests that called execute() but
    never checked the result (test_vendor_files_excluded,
    test_dockerfile_detection, test_github_actions_detection,
    test_makefile_detection, test_framework_detection).

Testing

  • Unit tests pass (make test-unit)
  • Integration tests pass (make test-integration)
  • Linter passes (make lint)
  • Type checker passes (make typecheck)
  • New/updated tests cover the changes

Verified the fix directly against the reproduction script from the issue
(reproduce_issue_150.py) — now returns primary_language = Python as
expected, instead of JavaScript.

Pre-existing failures: confirmed via branch comparison — main has 53 failing / 375 passing unit tests before this branch; this branch has 51 failing / 381 passing. All remaining 51 failures are in files unrelated to this issue (test_review_service.py, test_pii_scrubber.py, test_bias_detector.py, etc.) and exist identically on main. This PR fixes 2 test failures and adds 6 newly-passing assertions, all within tests/unit/test_tech_detector.py.

Screenshots / Demo

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

Notes for Reviewers

While fixing this, I found two separate, pre-existing bugs in
tech_detector.py that are out of scope for #150 but worth flagging:

  1. primary_language selection is alphabetical (sorted(languages)[0]),
    not frequency-based — e.g. a repo with 2 Python files and 1 Dockerfile
    reports "Infrastructure" as primary since it sorts before "Python".
  2. Extension-based language detection is actually case-sensitive
    (filepath.endswith(ext) without lowercasing filepath first), despite
    an existing test named test_case_insensitive_extension_matching.

I left both untouched here to keep this PR scoped to #150, but I'm happy
to file a follow-up issue for either/both if that's useful.

@sojsun17
sojsun17 marked this pull request as ready for review August 10, 2026 00:01
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