Skip to content

fix(ignore): scope default vendor exclusions to dependency manifests - #1081

Open
azizur100389 wants to merge 5 commits into
tirth8205:stagingfrom
azizur100389:codex/fix-vendor-ignore
Open

azizur100389 wants to merge 5 commits into
tirth8205:stagingfrom
azizur100389:codex/fix-vendor-ignore

Conversation

@azizur100389

@azizur100389 azizur100389 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Linked issue

Closes #1056.

What & why

The unconditional vendor default hides legitimate source packages in repositories that do not use vendor for dependencies, with no way to override it. Apply that default only to Composer, Go, and Ruby projects identified by root manifests (composer.json, go.mod, go.work, Gemfile, Rakefile, or *.gemspec). Explicit vendor/ entries in .code-review-graphignore still work, as do all other defaults. Keep DEFAULT_IGNORE_PATTERNS intact and document the repository-aware behavior.

How it was tested

Following the same staged regression/baseline methodology used in my earlier PRs (#274, #276, #279, #353 and #354), against the current contribution target staging at 6b12d11625cbec3b6773e076cb3d136464fa90e5.

  • New regression tests: 11/11 passed on Windows, Python 3.13.14. The reported bug reproduces on unchanged staging before the fix. The original Composer-only rule exposed three real Go/Ruby import regressions in CI; the corrected manifest-aware rule preserves those existing expectations without weakening their tests.
  • Initial affected-module run: 160 passed, 3 baseline failures. Every failure also reproduces on unchanged staging (compared by JUnit test identity). The final expanded regression module passed separately; no new affected-module failures.
  • Go/Ruby import-scope modules plus the new regression module: 70 passed, 7 existing Windows failures; the same seven fail on unchanged staging.
  • ruff check code_review_graph tests/test_composer_vendor_ignore.py: clean.
  • bandit -r code_review_graph/ -c pyproject.toml: zero findings.
  • mypy code_review_graph/ --ignore-missing-imports --no-strict-optional: the same three existing Windows fcntl.LOCK_EX/LOCK_NB/flock errors in daemon.py as unchanged staging; no new errors.
  • Schema remains version 13 on both Python and VS Code sides.
  • GitHub CI: 17 checks passed on the final commit. Python 3.10–3.13, browser tests, Windows native tests, and the Linux/macOS/Windows MCP client checks are green. Python 3.13 full-suite result: 4279 passed, 804 skipped, 46 deselected, 2 xfailed, 2 xpassed; 85.86% coverage (65% required). The manual upgrade job is skipped by design.

Exact test commands (using a local virtual environment with .[dev], as uv is not installed):

python -m pytest tests/test_composer_vendor_ignore.py -q --tb=short
python -m pytest tests/test_composer_vendor_ignore.py tests/test_incremental.py tests/test_watch_ignore_refresh.py -q --tb=short
$env:GIT_CEILING_DIRECTORIES = $env:TEMP.Replace('\', '/')
python -m pytest --tb=short -q -m 'not browser and not upgrade' --cov=code_review_graph --cov-report=term --cov-fail-under=65

Full local Windows suite completed: 4,154 passed, 110 failed, 819 skipped, 46 deselected, 2 xfailed, 2 xpassed; 86% coverage. Unchanged staging: 4,143 passed, 110 failed, with identical skip/xfail/xpass counts. Failure identities match unchanged staging exactly; no new failures. The Git ceiling isolates temporary test repositories from an unrelated enclosing home-directory Git repository; PATH/PYTHONPATH point to each checkout under test. Browser tests passed in CI; browser and opt-in upgrade suites were excluded locally.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.55 (MEDIUM) — 13 changed function(s)/class(es), 39 affected flow(s), 3 test gap(s) (1 with no tested caller found, 2 reached only through a caller)

Risk-scored changes

Risk Level Symbol Location Tested
0.55 medium code_review_graph/incremental.py::_load_ignore_patterns code_review_graph/incremental.py:545 yes
0.55 medium code_review_graph/incremental.py::_create_watch_handler code_review_graph/incremental.py:2915 yes
0.35 low code_review_graph/incremental.py::_scan_nested_output_dirs code_review_graph/incremental.py:648 indirect
0.35 low code_review_graph/incremental.py::WatchBatchProcessor code_review_graph/incremental.py:2930 indirect
0.35 low code_review_graph/incremental.py::WatchBatchProcessor._refresh_ignore_patterns code_review_graph/incremental.py:2968 no
0.25 low tests/test_composer_vendor_ignore.py::test_removing_root_manifest_invalidates_nested_scan tests/test_composer_vendor_ignore.py:75 (test)
0.05 low tests/test_composer_vendor_ignore.py::test_non_composer_vendor_source_is_visible tests/test_composer_vendor_ignore.py:17 (test)
0.05 low tests/test_composer_vendor_ignore.py::test_composer_dependencies_are_still_excluded tests/test_composer_vendor_ignore.py:22 (test)
0.05 low tests/test_composer_vendor_ignore.py::test_explicit_vendor_ignore_still_works_without_composer tests/test_composer_vendor_ignore.py:27 (test)
0.05 low tests/test_composer_vendor_ignore.py::test_other_dependency_defaults_are_unchanged tests/test_composer_vendor_ignore.py:32 (test)

Affected execution flows

  • process — criticality 0.74, 538 node(s) across 14 file(s)
  • _run — criticality 0.74, 565 node(s) across 16 file(s)
  • build_or_update_graph_tool — criticality 0.73, 619 node(s) across 24 file(s)
  • main — criticality 0.72, 925 node(s) across 40 file(s)
  • query_graph_tool — criticality 0.69, 97 node(s) across 11 file(s)
  • ...and 34 more affected flow(s)

Test gaps

  • code_review_graph/incremental.py::WatchBatchProcessor._refresh_ignore_patterns (code_review_graph/incremental.py:2968)

Reached only through a caller

No test names these directly, but a tested caller reaches them along the call path shown. That is a path in the call graph, not a record of execution -- the caller's tests may never take this branch. Treat it as where to look, not as coverage: these count as gaps and are scored as untested.

  • code_review_graph/incremental.py::_scan_nested_output_dirs (code_review_graph/incremental.py:648) — via code_review_graph/incremental.py::_load_ignore_patterns, 2 hop(s)
  • code_review_graph/incremental.py::WatchBatchProcessor (code_review_graph/incremental.py:2930) — via code_review_graph/incremental.py::_create_watch_handler, 1 hop(s)

Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

@azizur100389 azizur100389 changed the title fix(ignore): apply the default vendor exclusion only to Composer projects fix(ignore): scope default vendor exclusions to dependency manifests Oct 1, 2026
@azizur100389
azizur100389 marked this pull request as ready for review October 1, 2026 16:46
@tirth8205

Copy link
Copy Markdown
Owner

Thank you for this PR. I tested it with real repos, builds and watch mode. The Java vendor fix works well.

Before I can merge it, please fix these:

  1. Watch mode now adds a gitignored vendor/ in a subfolder. A full build skips it. Example: api/composer.json, "vendor/" in .gitignore, no root manifest. Run composer install while watch runs. All vendor files enter the graph. See code_review_graph/incremental.py:554.
  2. A Go import can point to a vendor/ that has no nodes. Example: server/go.mod, gitignored server/vendor, no root manifest. The import edge names server/vendor/github.com/ext/lib. See code_review_graph/parser.py:14847.
  3. Deleting a root Gemfile during watch keeps an old build-output scan. Then vendor/mod/target/ beside vendor/mod/pom.xml is indexed for up to 5 minutes. See code_review_graph/incremental.py:2912.

One fix may cover 1 and 2. Treat vendor/ next to composer.json, go.mod or Gemfile as dependencies. This works like target/ next to pom.xml.

After you push the fix, I will test it again.

@azizur100389

Copy link
Copy Markdown
Contributor Author

Hi @tirth8205, the review issues have been fixed and pushed, with regression tests covering the reported cases.

Nested vendor directories next to dependency manifests are now excluded consistently in watch mode and import resolution. Manifest changes also invalidate the cached scan, including deletion of a root Gemfile.

I rechecked CI today: all 17 checks are passing, with the upgrade-path check intentionally skipped. The branch is also up to date with staging and has no merge conflicts.

Could you please retest the updated PR when you have a chance? Thank you!

This branch has not been deployed

No deployments
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.

**/vendor/** is unconditional and a repository cannot take it back

2 participants